Skip to content

Commit

Permalink
spawn/ProcessHandle: add completion callback
Browse files Browse the repository at this point in the history
This allows callers to not only wait for completion, but also receive
error details.  This will replace lots of weird (protocol/socket)
error messages (because the child has disappeared and has reset a
socket connection to the caller) with spawner error messages.
  • Loading branch information
MaxKellermann committed Jul 8, 2024
1 parent 99cc2fd commit 77ef53f
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/spawn/Client.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "Client.hxx"
#include "ProcessHandle.hxx"
#include "CompletionHandler.hxx"
#include "IProtocol.hxx"
#include "Builder.hxx"
#include "Parser.hxx"
Expand Down Expand Up @@ -60,6 +61,8 @@ struct SpawnServerClient::ChildProcess final

const unsigned pid;

SpawnCompletionHandler *completion_handler = nullptr;

ExitListener *listener = nullptr;

ChildProcess(SpawnServerClient &_client, unsigned _pid) noexcept
Expand All @@ -70,6 +73,13 @@ struct SpawnServerClient::ChildProcess final
Kill(SIGTERM);
}

/* virtual methods from class ChildProcessHandle */
void SetCompletionHandler(SpawnCompletionHandler &_completion_handler) noexcept override {
assert(!completion_handler);

completion_handler = &_completion_handler;
}

void SetExitListener(ExitListener &_listener) noexcept override {
assert(is_linked());

Expand Down Expand Up @@ -512,7 +522,21 @@ SpawnServerClient::HandleExecCompleteMessage(SpawnPayload payload)
payload.ReadUnsigned(pid);
const char *error = payload.ReadString();

// TODO pass error condition to completion handler
if (const auto i = processes.find(pid); i != processes.end()) {
// TODO forward errors
if (i->completion_handler) {
if (*error == 0)
i->completion_handler->OnSpawnSuccess();
else
i->completion_handler->OnSpawnError(std::make_exception_ptr(std::runtime_error{error}));

/* if there is a completion handler,
don't log error message to
stderr */
continue;
}
}

if (*error != 0)
fmt::print(stderr, "Failed to spawn child process {}: {}\n",
pid, error);
Expand Down
18 changes: 18 additions & 0 deletions src/spawn/CompletionHandler.hxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: BSD-2-Clause
// Copyright CM4all GmbH
// author: Max Kellermann <[email protected]>

#pragma once

#include <exception>

/**
* This interface gets notified when spawning a child process
* completes. Pass a reference to an instance to
* ChildProcessHandle::SetCompletionHandler().
*/
class SpawnCompletionHandler {
public:
virtual void OnSpawnSuccess() noexcept = 0;
virtual void OnSpawnError(std::exception_ptr error) noexcept = 0;
};
7 changes: 7 additions & 0 deletions src/spawn/Local.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "Local.hxx"
#include "ProcessHandle.hxx"
#include "CompletionHandler.hxx"
#include "PidfdEvent.hxx"
#include "ExitListener.hxx"
#include "Config.hxx"
Expand Down Expand Up @@ -50,6 +51,12 @@ class LocalChildProcess final : public ChildProcessHandle, ExitListener {
void OnChildProcessExit(int status) noexcept override;

/* virtual methods from class ChildProcessHandle */
void SetCompletionHandler(SpawnCompletionHandler &handler) noexcept override {
assert(pidfd);

handler.OnSpawnSuccess();
}

void SetExitListener(ExitListener &listener) noexcept override {
assert(pidfd);

Expand Down
2 changes: 2 additions & 0 deletions src/spawn/ProcessHandle.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

class SpawnCompletionHandler;
class ExitListener;

/**
Expand All @@ -18,6 +19,7 @@ public:
*/
virtual ~ChildProcessHandle() noexcept = default;

virtual void SetCompletionHandler(SpawnCompletionHandler &handler) noexcept = 0;
virtual void SetExitListener(ExitListener &listener) noexcept = 0;

/**
Expand Down

0 comments on commit 77ef53f

Please sign in to comment.