workshop/Workplace: move stderr pipe initialization to WorkshopOperator::Start()

Author: MaxKellermann

src/workshop/Operator.cxx

@@ -68,24 +68,13 @@ class WorkshopOperator::SpawnedProcess final
 WorkshopOperator::WorkshopOperator(EventLoop &_event_loop,
 				   WorkshopWorkplace &_workplace,
 				   const WorkshopJob &_job,
-				   const std::shared_ptr<Plan> &_plan,
-				   UniqueFileDescriptor stderr_read_pipe,
-				   size_t max_log_buffer,
-				   bool enable_journal) noexcept
+				   const std::shared_ptr<Plan> &_plan) noexcept
 	:event_loop(_event_loop),
 	 workplace(_workplace), job(_job), plan(_plan),
 	 logger(*this),
-	 timeout_event(event_loop, BIND_THIS_METHOD(OnTimeout)),
-	 log(event_loop, job.plan_name, job.id,
-	     std::move(stderr_read_pipe))
+	 timeout_event(event_loop, BIND_THIS_METHOD(OnTimeout))
 {
 	ScheduleTimeout();
-
-	if (max_log_buffer > 0)
-		log.EnableBuffer(max_log_buffer);
-
-	if (enable_journal)
-		log.EnableJournal();
 }
 
 WorkshopOperator::~WorkshopOperator() noexcept
@@ -127,10 +116,24 @@ PrepareChildProcess(PreparedChildProcess &p, const char *plan_name,
 }
 
 void
-WorkshopOperator::Start(FileDescriptor stderr_w)
+WorkshopOperator::Start(std::size_t max_log_buffer,
+			bool enable_journal)
 {
 	auto &spawn_service = workplace.GetSpawnService();
 
+	/* create stdout/stderr pipes */
+
+	auto [stderr_r, stderr_w] = CreatePipe();
+	stderr_r.SetNonBlocking();
+
+	log.emplace(event_loop, job.plan_name, job.id, std::move(stderr_r));
+
+	if (max_log_buffer > 0)
+		log->EnableBuffer(max_log_buffer);
+
+	if (enable_journal)
+		log->EnableJournal();
+
 	if (plan->control_channel && plan->allow_spawn)
 		stderr_write_pipe = stderr_w.Duplicate();
 
@@ -314,6 +317,8 @@ WorkshopOperator::Expand(std::list<std::string> &args) const noexcept
 void
 WorkshopOperator::OnChildProcessExit(int status) noexcept
 {
+	assert(log);
+
 	exited = true;
 
 	if (control_channel) {
@@ -325,7 +330,7 @@ WorkshopOperator::OnChildProcessExit(int status) noexcept
 		}
 	}
 
-	log.Flush();
+	log->Flush();
 
 	int exit_status = WEXITSTATUS(status);
 
@@ -342,7 +347,7 @@ WorkshopOperator::OnChildProcessExit(int status) noexcept
 	else
 		logger(2, "exited with status ", exit_status);
 
-	const char *log_text = log.GetBuffer();
+	const char *log_text = log->GetBuffer();
 	if (log_text != nullptr && !ValidateUTF8(log_text)) {
 		/* TODO: purge illegal UTF-8 sequences instead of
 		   replacing the log text? */

src/workshop/Operator.hxx

@@ -2,8 +2,7 @@
 // Copyright CM4all GmbH
 // author: Max Kellermann <mk@cm4all.com>
 
-#ifndef WORKSHOP_OPERATOR_HXX
-#define WORKSHOP_OPERATOR_HXX
+#pragma once
 
 #include "Job.hxx"
 #include "LogBridge.hxx"
@@ -16,6 +15,7 @@
 #include "util/IntrusiveList.hxx"
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <list>
 #include <chrono>
@@ -74,7 +74,7 @@ class WorkshopOperator final
 	 */
 	UniqueFileDescriptor cgroup_cpu_stat;
 
-	LogBridge log;
+	std::optional<LogBridge> log;
 
 	class SpawnedProcess;
 
@@ -87,10 +87,7 @@ class WorkshopOperator final
 public:
 	WorkshopOperator(EventLoop &_event_loop,
 			 WorkshopWorkplace &_workplace, const WorkshopJob &_job,
-			 const std::shared_ptr<Plan> &_plan,
-			 UniqueFileDescriptor stderr_read_pipe,
-			 size_t max_log_buffer,
-			 bool enable_journal) noexcept;
+			 const std::shared_ptr<Plan> &_plan) noexcept;
 
 	WorkshopOperator(const WorkshopOperator &other) = delete;
 
@@ -106,7 +103,8 @@ public:
 		return job.plan_name;
 	}
 
-	void Start(FileDescriptor stderr_w);
+	void Start(std::size_t max_log_buffer,
+		   bool enable_journal);
 
 	void SetPid(std::unique_ptr<ChildProcessHandle> &&_pid) noexcept {
 		pid = std::move(_pid);
@@ -142,5 +140,3 @@ private:
 	void OnControlPermanentError(std::exception_ptr &&error) noexcept override;
 	void OnControlClosed() noexcept override;
 };
-
-#endif

src/workshop/Workplace.cxx

@@ -91,18 +91,10 @@ WorkshopWorkplace::Start(EventLoop &event_loop, const WorkshopJob &job,
 {
 	assert(!plan->args.empty());
 
-	/* create stdout/stderr pipes */
-
-	auto [stderr_r, stderr_w] = CreatePipe();
-	stderr_r.SetNonBlocking();
-
 	/* create operator object */
 
-	auto o = std::make_unique<WorkshopOperator>(event_loop, *this, job, std::move(plan),
-						    std::move(stderr_r),
-						    max_log,
-						    enable_journal);
-	o->Start(stderr_w);
+	auto o = std::make_unique<WorkshopOperator>(event_loop, *this, job, std::move(plan));
+	o->Start(max_log, enable_journal);
 
 	operators.push_back(*o.release());
 }