From 99f41c28cbe4b95998f025e32d90813e65ceef90 Mon Sep 17 00:00:00 2001
From: Lion Kortlepel <development@kortlepel.com>
Date: Fri, 28 Oct 2022 17:42:10 +0200
Subject: [PATCH] fix #135 by making onPlayerDisconnect blocking, and calling
 it before removing the player

before, the handlers were not waited for, so the client was usually
destructed before lua got to the actual event handler call. Now, the
handler is called and waited on, and once all handlers are done, the
client is properly removed from the players internally, thus making
calls to GetPlayerName, GetPlayerIdentifiers, etc. return nil etc.
---
 src/Client.cpp   |  1 +
 src/TNetwork.cpp | 12 +++++++++---
 src/TServer.cpp  | 20 ++++++++++++++------
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/Client.cpp b/src/Client.cpp
index b03c99a..047c92c 100644
--- a/src/Client.cpp
+++ b/src/Client.cpp
@@ -6,6 +6,7 @@
 #include <optional>
 
 void TClient::DeleteCar(int Ident) {
+    // TODO: Send delete packets
     std::unique_lock lock(mVehicleDataMutex);
     auto iter = std::find_if(mVehicleData.begin(), mVehicleData.end(), [&](auto& elem) {
         return Ident == elem.ID();
diff --git a/src/TNetwork.cpp b/src/TNetwork.cpp
index 6fbc16c..bfc9302 100644
--- a/src/TNetwork.cpp
+++ b/src/TNetwork.cpp
@@ -522,8 +522,14 @@ void TNetwork::UpdatePlayer(TClient& Client) {
 }
 
 void TNetwork::OnDisconnect(const std::weak_ptr<TClient>& ClientPtr) {
-    beammp_assert(!ClientPtr.expired());
-    auto LockedClientPtr = ClientPtr.lock();
+    std::shared_ptr<TClient> LockedClientPtr { nullptr };
+    try {
+        LockedClientPtr = ClientPtr.lock();
+    } catch (const std::exception&) {
+        // do nothing ig
+        return;
+    }
+    beammp_assert(LockedClientPtr != nullptr);
     TClient& c = *LockedClientPtr;
     beammp_info(c.GetName() + (" Connection Terminated"));
     std::string Packet;
@@ -540,7 +546,7 @@ void TNetwork::OnDisconnect(const std::weak_ptr<TClient>& ClientPtr) {
     SendToAll(&c, StringToVector(Packet), false, true);
     Packet.clear();
     auto Futures = LuaAPI::MP::Engine->TriggerEvent("onPlayerDisconnect", "", c.GetID());
-    LuaAPI::MP::Engine->ReportErrors(Futures);
+    LuaAPI::MP::Engine->WaitForAll(Futures);
     c.Disconnect("Already Disconnected (OnDisconnect)");
     mServer.RemoveClient(ClientPtr);
 }
diff --git a/src/TServer.cpp b/src/TServer.cpp
index f160e6a..0a144f8 100644
--- a/src/TServer.cpp
+++ b/src/TServer.cpp
@@ -1,6 +1,7 @@
 #include "TServer.h"
 #include "Client.h"
 #include "Common.h"
+#include "CustomAssert.h"
 #include "TNetwork.h"
 #include "TPPSMonitor.h"
 #include <TLuaPlugin.h>
@@ -94,13 +95,20 @@ TServer::TServer(const std::vector<std::string_view>& Arguments) {
 }
 
 void TServer::RemoveClient(const std::weak_ptr<TClient>& WeakClientPtr) {
-    if (!WeakClientPtr.expired()) {
-        TClient& Client = *WeakClientPtr.lock();
-        beammp_debug("removing client " + Client.GetName() + " (" + std::to_string(ClientCount()) + ")");
-        Client.ClearCars();
-        WriteLock Lock(mClientsMutex);
-        mClients.erase(WeakClientPtr.lock());
+    std::shared_ptr<TClient> LockedClientPtr { nullptr };
+    try {
+        LockedClientPtr = WeakClientPtr.lock();
+    } catch (const std::exception&) {
+        // silently fail, as there's nothing to do
+        return;
     }
+    beammp_assert(LockedClientPtr != nullptr);
+    TClient& Client = *LockedClientPtr;
+    beammp_debug("removing client " + Client.GetName() + " (" + std::to_string(ClientCount()) + ")");
+    // TODO: Send delete packets for all cars
+    Client.ClearCars();
+    WriteLock Lock(mClientsMutex);
+    mClients.erase(WeakClientPtr.lock());
 }
 
 void TServer::ForEachClient(const std::function<bool(std::weak_ptr<TClient>)>& Fn) {