Revert "Use ref in ComponentThread::joltThreadReq"

This reverts commit 2222491c21.

The thread lifetime ops need to use sh_ptrs because apparently the
thread objects go out of scope at some point during shutdown, before
the threads can actually finish shutting down.

libspinscale/include/spinscale/componentThread.h

@@ -125,8 +125,14 @@ public:
 	 * JOLTing is the mechanism that allows threads to enter their main
 	 * event loops and set up TLS vars after all global constructors have
 	 * completed. This prevents race conditions during system startup.
+	 *
+	 * @param selfPtr Shared pointer to this thread (required because TLS
+	 *                isn't set up yet, so shared_from_this() can't be used)
+	 * @param callback Callback to invoke when JOLT completes
 	 */
-	void joltThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback);
+	void joltThreadReq(
+		const std::shared_ptr<PuppetThread>& selfPtr,
+		Callback<threadLifetimeMgmtOpCbFn> callback);
 
 	// CPU management methods
 	void pinToCpu(int cpuId);

libspinscale/src/componentThread.cpp

@@ -66,7 +66,7 @@ class PuppetThread::ThreadLifetimeMgmtOp
 public:
 	ThreadLifetimeMgmtOp(
 		const std::shared_ptr<ComponentThread> &caller,
-		PuppetThread& target,
+		const std::shared_ptr<PuppetThread> &target,
 		Callback<threadLifetimeMgmtOpCbFn> callback)
 	:	PostedAsynchronousContinuation<threadLifetimeMgmtOpCbFn>(
 			caller, callback),
@@ -74,18 +74,18 @@ public:
 	{}
 
 public:
-	PuppetThread& target;
+	const std::shared_ptr<PuppetThread> target;
 
 public:
 	void joltThreadReq1_posted(
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"JOLT request."
 			<< "\n";
 
-		target.io_service.stop();
+		target->io_service.stop();
 		callOriginalCb();
 	}
 
@@ -93,7 +93,7 @@ public:
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"startThread."
 			<< "\n";
 
@@ -107,11 +107,11 @@ public:
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"exitThread (main queue)." << "\n";
 
-		target.cleanup();
-		target.io_service.stop();
+		target->cleanup();
+		target->io_service.stop();
 		callOriginalCb();
 	}
 
@@ -119,12 +119,12 @@ public:
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"exitThread (pause queue)."<< "\n";
 
-		target.cleanup();
-		target.pause_io_service.stop();
-		target.io_service.stop();
+		target->cleanup();
+		target->pause_io_service.stop();
+		target->io_service.stop();
 		callOriginalCb();
 	}
 
@@ -132,7 +132,7 @@ public:
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"pauseThread." << "\n";
 
 		/* We have to invoke the callback here before moving on because 
@@ -140,18 +140,18 @@ public:
 		 * have a chance to invoke the callback until it's unblocked.
 		 */
 		callOriginalCb();
-		target.pause_io_service.reset();
-		target.pause_io_service.run();
+		target->pause_io_service.reset();
+		target->pause_io_service.run();
 	}
 
 	void resumeThreadReq1_posted(
 		[[maybe_unused]] std::shared_ptr<ThreadLifetimeMgmtOp> context
 		)
 	{
-		std::cout << __func__ << ": Thread '" << target.name << "': handling "
+		std::cout << __func__ << ": Thread '" << target->name << "': handling "
 			"resumeThread." << "\n";
 
-		target.pause_io_service.stop();
+		target->pause_io_service.stop();
 		callOriginalCb();
 	}
 };
@@ -161,7 +161,9 @@ void ComponentThread::cleanup(void)
 	this->keepLooping = false;
 }
 
-void PuppetThread::joltThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
+void PuppetThread::joltThreadReq(
+	const std::shared_ptr<PuppetThread>& selfPtr,
+	Callback<threadLifetimeMgmtOpCbFn> callback)
 {
 	/**	EXPLANATION:
 	 * We can't use shared_from_this() here because JOLTing occurs prior to
@@ -175,9 +177,8 @@ void PuppetThread::joltThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 	 * CRT main() function invokes on the mrntt thread is special since it
 	 * supplies cmdline args and envp.
 	 *
-	 * To obtain a ref to the target thread, we just use 'this'. Since
-	 * the operation is posted to this thread's io_service (which is a member
-	 * of this object), the object must be alive when the operation executes.
+	 * To obtain a sh_ptr to the target thread, we use the selfPtr parameter
+	 * passed in by the caller.
 	 */
 	if (id == sscl::marionetteThreadId)
 	{
@@ -188,7 +189,7 @@ void PuppetThread::joltThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 	std::shared_ptr<MarionetteThread> mrntt = smo::mrntt::thread;
 
 	auto request = std::make_shared<ThreadLifetimeMgmtOp>(
-		mrntt, *this, callback);
+		mrntt, selfPtr, callback);
 
 	this->getIoService().post(
 		STC(std::bind(
@@ -201,7 +202,8 @@ void PuppetThread::startThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 {
 	std::shared_ptr<ComponentThread> caller = getSelf();
 	auto request = std::make_shared<ThreadLifetimeMgmtOp>(
-		caller, *this, callback);
+		caller, std::static_pointer_cast<PuppetThread>(shared_from_this()),
+		callback);
 
 	this->getIoService().post(
 		STC(std::bind(
@@ -213,7 +215,8 @@ void PuppetThread::exitThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 {
 	std::shared_ptr<ComponentThread> caller = getSelf();
 	auto request = std::make_shared<ThreadLifetimeMgmtOp>(
-		caller, *this, callback);
+		caller, std::static_pointer_cast<PuppetThread>(shared_from_this()),
+		callback);
 
 	this->getIoService().post(
 		STC(std::bind(
@@ -236,7 +239,8 @@ void PuppetThread::pauseThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 
 	std::shared_ptr<ComponentThread> caller = getSelf();
 	auto request = std::make_shared<ThreadLifetimeMgmtOp>(
-		caller, *this, callback);
+		caller, std::static_pointer_cast<PuppetThread>(shared_from_this()),
+		callback);
 
 	this->getIoService().post(
 		STC(std::bind(
@@ -255,7 +259,8 @@ void PuppetThread::resumeThreadReq(Callback<threadLifetimeMgmtOpCbFn> callback)
 	// Post to the pause_io_service to unblock the paused thread
 	std::shared_ptr<ComponentThread> caller = getSelf();
 	auto request = std::make_shared<ThreadLifetimeMgmtOp>(
-		caller, *this, callback);
+		caller, std::static_pointer_cast<PuppetThread>(shared_from_this()),
+		callback);
 
 	pause_io_service.post(
 		STC(std::bind(

libspinscale/src/puppetApplication.cpp

@@ -99,6 +99,7 @@ void PuppetApplication::joltAllPuppetThreadsReq(
 	for (auto& thread : componentThreads)
 	{
 		thread->joltThreadReq(
+			thread,
 			{request, std::bind(
 				&PuppetThreadLifetimeMgmtOp::joltAllPuppetThreadsReq1,
 				request.get(), request)});