Refactor to use LockedWrapper guard

Source/Integration/IntegrationGameModeBase.cpp

@@ -41,10 +41,10 @@ uint32 AIntegrationGameModeBase::Run()
 			continue;
 		}
 		{
-			FScopeLock lk(&LuprexMutex);
-			Sockets->Update();
-			Luprex.play_invoke_event_update(&Luprex, EngineSeconds);
-			Sockets->Update();
+			FLockedWrapper lockedwrap(LockableWrapper);
+			Sockets->Update(lockedwrap);
+			lockedwrap->play_invoke_event_update(lockedwrap.Get(), EngineSeconds);
+			Sockets->Update(lockedwrap);
 		}
 	}
 	return 0;
@@ -52,6 +52,8 @@ uint32 AIntegrationGameModeBase::Run()
 
 void AIntegrationGameModeBase::ResetToInitialState()
 {
+	FLockedWrapper w(LockableWrapper);
+
 	// Shut down the thread and release the ThreadEvent
 	if (Thread != nullptr)
 	{
@@ -65,12 +67,16 @@ void AIntegrationGameModeBase::ResetToInitialState()
 	ThreadStopRequested = false;
 
 	// Release and close all sockets.
-	Sockets.Reset();
+	if (Sockets != nullptr)
+	{
+		Sockets->ForceCloseEverything(w);
+		Sockets.Reset();
+	}
 
 	// Delete the engine.
-	if (Luprex.release != nullptr)
+	if (w->release != nullptr)
 	{
-		Luprex.release(&Luprex);
+		w->release(w.Get());
 	}
 
 	// Reset the clocks.
@@ -79,15 +85,15 @@ void AIntegrationGameModeBase::ResetToInitialState()
 }
 
 
-void AIntegrationGameModeBase::HandleLuprexConsoleOutput()
+void AIntegrationGameModeBase::HandleLuprexConsoleOutput(FLockedWrapper &w)
 {
 	uint32_t ndata; const char* data;
-	Luprex.get_outgoing(&Luprex, 0, &ndata, &data);
+	w->get_outgoing(w.Get(), 0, &ndata, &data);
 	if (ndata == 0) return;
 	std::string_view src(data, ndata);
 	int consumed;
 	std::u16string cps = drvutil::utf8_to_ucs2(src, &consumed);
-	Luprex.play_sent_outgoing(&Luprex, 0, consumed);
+	w->play_sent_outgoing(w.Get(), 0, consumed);
 	FString fs(cps.size(), (const UCS2CHAR*)(&cps[0]));
 	ConsoleOutput.Append(fs);
 }
@@ -96,11 +102,11 @@ void AIntegrationGameModeBase::Tick(float DeltaSeconds)
 {
 	Super::Tick(DeltaSeconds);
 	{
-		FScopeLock lk(&LuprexMutex);
-		if (Luprex.engine != nullptr)
+		FLockedWrapper lockedwrap(LockableWrapper);
+		if (lockedwrap->engine != nullptr)
 		{
 			EngineSeconds += DeltaSeconds;
-			HandleLuprexConsoleOutput();
+			HandleLuprexConsoleOutput(lockedwrap);
 		}
 	}
 	TArray<FString> prints = engineutil::DPrintGetStored();
@@ -121,9 +127,9 @@ void AIntegrationGameModeBase::Tick(float DeltaSeconds)
 
 void AIntegrationGameModeBase::ConsoleSendInput(const FString& fs)
 {
-	if (Luprex.engine != nullptr)
+	FLockedWrapper w(LockableWrapper);
+	if (w->engine != nullptr)
 	{
-		FScopeLock lk(&LuprexMutex);
 		const TCHAR* fstchar = *fs;
 		if (sizeof(TCHAR) == 2)
 		{
@@ -131,7 +137,7 @@ void AIntegrationGameModeBase::ConsoleSendInput(const FString& fs)
 			std::u16string_view fsview((const char16_t*)fstchar, fs.Len());
 			std::string utf8 = drvutil::utf16_to_utf8(fsview);
 			utf8 = utf8 + "\n";
-			Luprex.play_recv_incoming(&Luprex, 0, utf8.size(), utf8.c_str());
+			w->play_recv_incoming(w.Get(), 0, utf8.size(), utf8.c_str());
 		}
 	}
 }
@@ -140,27 +146,26 @@ void AIntegrationGameModeBase::BeginPlay()
 {
 	Super::BeginPlay();
 
+	FLockedWrapper w(LockableWrapper);
+
 	// Sanity checks.
 	checkf(Thread == nullptr, TEXT("There should be no thread here."));
-	checkf(Luprex.engine == nullptr, TEXT("There should be no engine here."));
+	checkf(w->engine == nullptr, TEXT("There should be no engine here."));
 
 	// Make sure we're starting from a clean slate.
 	ResetToInitialState();
 
 	// Try to initialize the wrapper.
-	if (Luprex.play_initialize == nullptr)
-	{
-		engineutil::init_wrapper(&Luprex);
-	}
+	w.InitWrapper();
 
 	// If we failed to initialize the wrapper, print an error message.
-	if (Luprex.play_initialize == nullptr)
+	if (w->play_initialize == nullptr)
 	{
 		engineutil::DPrint("Luprex wrapper initialization failed");
 	}
 
 	// If wrapper is initialized, try to initialize the luprex engine.
-	if (Luprex.play_initialize != nullptr)
+	if (w->play_initialize != nullptr)
 	{
 		drvutil::ostringstream srcpak;
 		std::string srcpakerr = drvutil::package_lua_source("c:\\Luprex", &srcpak);
@@ -171,10 +176,10 @@ void AIntegrationGameModeBase::BeginPlay()
 		std::string_view srcpakv = srcpak.view();
 		char* argv[1];
 		argv[0] = const_cast<char*>("lpxserver");
-		Luprex.play_initialize(&Luprex, 1, argv, srcpakv.size(), srcpakv.data(), "");
-		if (Luprex.error[0])
+		w->play_initialize(w.Get(), 1, argv, srcpakv.size(), srcpakv.data(), "");
+		if (w->error[0])
 		{
-			engineutil::DPrint(Luprex.error);
+			engineutil::DPrint(w->error);
 		}
 		else
 		{
@@ -183,9 +188,9 @@ void AIntegrationGameModeBase::BeginPlay()
 	}
 
 	// If we successfully created a luprex engine, create a socket system and a worker thread.
-	if (Luprex.engine != nullptr)
+	if (w->engine != nullptr)
 	{
-		Sockets.Reset(FLpxSockets::Create(&Luprex));
+		Sockets.Reset(FLpxSockets::Create(w));
 		std::string error = Sockets->GetError();
 		check(error.empty());
 		ThreadEvent = FPlatformProcess::GetSynchEventFromPool(false);

Source/Integration/IntegrationGameModeBase.h

@@ -45,7 +45,7 @@ public:
 	TSubclassOf<AActor> ClassTangibleActor;
 
 	// Transfer console output from the Luprex engine to unreal.
-	void HandleLuprexConsoleOutput();
+	void HandleLuprexConsoleOutput(FLockedWrapper &w);
 
 	UPROPERTY()
 	FTangibleManager TangibleManager;
@@ -65,11 +65,9 @@ public:
 	// to exit.
 	FEvent* ThreadEvent;
 
-	// This critical section guards the use of the EngineWrapper.
-	FCriticalSection LuprexMutex;
-	
-	// The Luprex wrapper and engine.  MUST CLAIM LuprexMutex.
-	EngineWrapper Luprex;
+	// The Luprex EngineWrapper, with a Mutex to protect it.
+	// To access it, construct a FLockedWrapper.
+	FLockableWrapper LockableWrapper;
 
 	// Luprex socket system.  Aside from construction, only touched by Luprex thread.
 	TUniquePtr<FLpxSockets> Sockets;

Source/Integration/LockedWrapper.cpp

@@ -0,0 +1,19 @@
+
+#include "LockedWrapper.h"
+#include "engineutil.hpp"
+
+void FLockedWrapper::InitWrapper() {
+	if (Lockable.Wrapper.play_initialize != nullptr) {
+		// Already initialized.
+		return;
+	}
+	void* DLL = FPlatformProcess::GetDllHandle(TEXT("c:\\Luprex\\build\\visual\\luprexlib.dll"));
+	if (DLL != nullptr) {
+		using InitFn = void (*)(EngineWrapper*);
+		InitFn init = (InitFn)FPlatformProcess::GetDllExport(DLL, TEXT("init_engine_wrapper"));
+		if (init != nullptr) {
+			init(&Lockable.Wrapper);
+			Lockable.Wrapper.hook_dprint(engineutil::DPrintHook);
+		}
+	}
+}

Source/Integration/LockedWrapper.h

@@ -0,0 +1,50 @@
+#pragma once
+
+#include "CoreMinimal.h"
+#include "enginewrapper.hpp"
+
+// Class FLockableWrapper
+// 
+// Contains the EngineWrapper and a Mutex to lock it.
+// This class has no methods.  To access the EngineWrapper,
+// construct a FLockedWrapper, and then dereference it
+// using operator right arrow.
+//
+class FLockableWrapper {
+private:
+	FCriticalSection Mutex;
+	EngineWrapper Wrapper;
+public:
+	friend class FLockedWrapper;
+};
+
+class FLockedWrapper {
+private:
+	FLockableWrapper& Lockable;
+
+public:
+	// The constructor of the FLockedWrapper claims the mutex.
+	FLockedWrapper(FLockableWrapper& w) : Lockable(w) {
+		Lockable.Mutex.Lock();
+	}
+
+	// The destructor of the FLockedWrapper releases the mutex.
+	~FLockedWrapper() {
+		Lockable.Mutex.Unlock();
+	}
+
+	// Initialize the engine wrapper if it's not already.
+	void InitWrapper();
+
+	// Operator right arrow accesses the EngineWrapper.
+	EngineWrapper* operator ->() {
+		return &Lockable.Wrapper;
+	}
+
+	// Get a pointer to the enginewrapper.  This is not
+	// very safe because you could keep the pointer after
+	// the LockedWrapper is destroyed.  Don't do that.
+	EngineWrapper* Get() {
+		return &Lockable.Wrapper;
+	}
+};

Source/Integration/LuprexSockets.cpp

@@ -172,7 +172,7 @@ public:
 
     FLpxChannel(FLpxSocketsI *lsi, FSocket* sock, int chid, SSL_CTX* ctx, EChanState state);
     FLpxChannel() : FLpxChannel(nullptr, nullptr, 0, nullptr, CHAN_INACTIVE) {}
-    ~FLpxChannel() { Close(""); }
+    ~FLpxChannel() { }
 };
 
 /////////////////////////////////////////////////////////////////
@@ -187,8 +187,8 @@ public:
     // Fatal error status.
     std::string FatalError;
 
-    // We don't own the wrapper, we just have a pointer to it.
-    // We require a guarantee that it outlives us.
+    // This pointer is NULL except when inside
+    // one of the methods that accepts a LockedWrapper.
     EngineWrapper* Luprex;
 
     // A general-purpose character buffer.
@@ -206,7 +206,7 @@ public:
     SSL_CTX* ClientSecureCTX;
     SSL_CTX* ClientInsecureCTX;
 
-    FLpxSocketsI(EngineWrapper* w);
+    FLpxSocketsI(FLockedWrapper &w);
     virtual ~FLpxSocketsI() override;
 
     // Copy the trace to the DPrint output.
@@ -226,8 +226,11 @@ public:
     void HandleNewOutgoingSockets();
     void HandleSocketInputOutput();
 
+    // Force Close Everything.
+    virtual void ForceCloseEverything(FLockedWrapper& w);
+
     // Main update routine.
-    virtual void Update() override;
+    virtual void Update(FLockedWrapper &w) override;
 };
 
 /////////////////////////////////////////////////////////////////
@@ -807,8 +810,11 @@ void FLpxSocketsI::SetError(const std::string& s)
 }
 
 
-FLpxSocketsI::FLpxSocketsI(EngineWrapper *w)
+FLpxSocketsI::FLpxSocketsI(FLockedWrapper &w)
 {
+    // We retain this pointer only so long as we have the wrapper lock.
+    Luprex = w.Get();
+
     // This function is nonreentrant.  It's not clear whether
     // this is needed - it may be initialized elsewhere in unreal.
     // It is also not clear that it's safe to do this in the
@@ -816,7 +822,6 @@ FLpxSocketsI::FLpxSocketsI(EngineWrapper *w)
     // thread).
     SSL_library_init();
 
-    Luprex = w;
     ServerCTX = nullptr;
     ClientSecureCTX = nullptr;
     ClientInsecureCTX = nullptr;
@@ -845,27 +850,52 @@ FLpxSocketsI::FLpxSocketsI(EngineWrapper *w)
         names = names + " " + name;
     }
     HandleListenPorts();
+
+    // We're losing the wrapper lock, so set the pointer to nullptr.
+    Luprex = nullptr;
+}
+
+void FLpxSocketsI::ForceCloseEverything(FLockedWrapper& w)
+{
+    // We retain this pointer only so long as we have the wrapper lock.
+    Luprex = w.Get();
+
+    // Close all channels
+    for (FLpxChannel& chan : Channels) {
+        chan.Close("Force Close Everything");
+    }
+
+    // Delete any channels released by the above.
+    RemoveInactiveChannels();
+
+    // All channels should be gone now.
+    check(Channels.IsEmpty());
+
+    // We're losing the wrapper lock, so set the pointer to nullptr.
+    Luprex = nullptr;
 }
 
 FLpxSocketsI::~FLpxSocketsI()
 {
+    checkf(Channels.IsEmpty(), TEXT("Must call ForceCloseEverything before destructor"));
+
     if (ServerCTX != nullptr)
     {
         SSL_CTX_free(ServerCTX);
+        ServerCTX = nullptr;
     }
     if (ClientSecureCTX != nullptr)
     {
         SSL_CTX_free(ClientSecureCTX);
+        ClientSecureCTX = nullptr;
     }
     if (ClientInsecureCTX != nullptr)
     {
         SSL_CTX_free(ClientInsecureCTX);
+        ClientInsecureCTX = nullptr;
     }
 
-    // Cleanup
-    //for (ChanInfo& chan : chans_) {
-    //    close_channel(chan, "");
-    //}
+    // TODO: Be more thorough.
 }
 
 void FLpxSocketsI::DPrintTrace()
@@ -988,13 +1018,19 @@ void FLpxSocketsI::HandleSocketInputOutput()
     RemoveInactiveChannels();
 }
 
-void FLpxSocketsI::Update()
+void FLpxSocketsI::Update(FLockedWrapper &w)
 {
+    // We retain this pointer only so long as we have the wrapper lock.
+    Luprex = w.Get();
+
     HandleNewOutgoingSockets();
     HandleSocketInputOutput();
+
+    // We're losing the wrapper lock, so set the pointer to nullptr.
+    Luprex = nullptr;
 }
 
-FLpxSockets* FLpxSockets::Create(EngineWrapper* w)
+FLpxSockets* FLpxSockets::Create(FLockedWrapper &w)
 {
 	return new FLpxSocketsI(w);
 }

Source/Integration/LuprexSockets.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "CoreMinimal.h"
+#include "LockedWrapper.h"
 #include <string>
 
 // Class FLpxSockets
@@ -33,15 +34,19 @@ protected:
 
 public:
 	// Create the Luprex socket system.
-	static FLpxSockets* Create(EngineWrapper *w);
+	static FLpxSockets* Create(FLockedWrapper &w);
 
-	// Delete the luprex socket system.  Cleanly closes all sockets.
+	// Close all sockets.
+	virtual void ForceCloseEverything(FLockedWrapper& w) = 0;
+
+	// The update routine relays data into and out of
+	// luprex via TCP/IP sockets.
+	virtual void Update(FLockedWrapper& w) = 0;
+
+	// Delete the luprex socket system.
+	// You must call ForceCloseEverything first.
 	virtual ~FLpxSockets() {}
 
 	// Obtain any error status report.
 	virtual std::string GetError() = 0;
-
-	// The update routine relays data into and out of
-	// luprex via TCP/IP sockets.
-	virtual void Update() = 0;
 };

Source/Integration/engineutil.cpp

@@ -3,17 +3,6 @@
 
 namespace engineutil {
 
-void init_wrapper(EngineWrapper* w) {
-	void* DLL = FPlatformProcess::GetDllHandle(TEXT("c:\\Luprex\\build\\visual\\luprexlib.dll"));
-	if (DLL != nullptr) {
-		using InitFn = void (*)(EngineWrapper*);
-		InitFn init = (InitFn)FPlatformProcess::GetDllExport(DLL, TEXT("init_engine_wrapper"));
-		if (init != nullptr) {
-			init(w);
-			w->hook_dprint(engineutil::DPrintHook);
-		}
-	}
-}
 
 // The DPrint array.  This stores the dprints
 // until they can be collected by the console implementation.