From 341fb8262d814f04055807ad419ce93b63944428 Mon Sep 17 00:00:00 2001
From: kaetemi <jan.boon@kaetemi.be>
Date: Sat, 4 Mar 2023 15:35:23 +0800
Subject: [PATCH] Fix stray reference to QUIC user context in outbox, ref
 ryzom/ryzomcore#628

---
 .../server/src/frontend_service/fe_send_sub.h |  7 ++--
 .../src/frontend_service/quic_transceiver.cpp |  7 ++--
 .../src/frontend_service/quic_transceiver.h   | 32 +++++++++++++++++--
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/ryzom/server/src/frontend_service/fe_send_sub.h b/ryzom/server/src/frontend_service/fe_send_sub.h
index 0b41af46a9..0a430d63df 100644
--- a/ryzom/server/src/frontend_service/fe_send_sub.h
+++ b/ryzom/server/src/frontend_service/fe_send_sub.h
@@ -144,8 +144,11 @@ class CFeSendSub
 	void					disableSendBuffer( TClientId id )
 	{
 		// We can disable both, because no problem if the flushing thread sees the state of a buffer change to unused
-		((*_CurrentFillingBuffers)[id]).enableSendBuffer( false );
-		((*_CurrentFlushingBuffers)[id]).enableSendBuffer( false );
+		// ((*_CurrentFillingBuffers)[id]).enableSendBuffer( false );
+		// ((*_CurrentFlushingBuffers)[id]).enableSendBuffer( false );
+		NLNET::CInetAddress nullAddr(false);
+		((*_CurrentFillingBuffers)[id]).setAddress( &nullAddr, NULL, false );
+		((*_CurrentFlushingBuffers)[id]).setAddress( &nullAddr, NULL, false );
 
 		// But we must remove the id for _BuffersToEnable in the case when there was
 		// a connection and then a disconnection for the same client in the same cycle
diff --git a/ryzom/server/src/frontend_service/quic_transceiver.cpp b/ryzom/server/src/frontend_service/quic_transceiver.cpp
index 45a410def7..e4360446df 100644
--- a/ryzom/server/src/frontend_service/quic_transceiver.cpp
+++ b/ryzom/server/src/frontend_service/quic_transceiver.cpp
@@ -437,12 +437,12 @@ void CQuicTransceiver::release()
 
 CQuicUserContext::CQuicUserContext()
 {
-	nldebug("Create QUIC user context, total %i", (int)(s_UserContextCount++));
+	nldebug("Create QUIC user context, total %i", (int)(++s_UserContextCount));
 }
 
 CQuicUserContext::~CQuicUserContext()
 {
-	nldebug("Destroy QUIC user context, total %i", (int)(s_UserContextCount--));
+	nldebug("Destroy QUIC user context, total %i", (int)(--s_UserContextCount));
 	
 	// This should never get called before the connection is shutdown,
 	// since we increase the reference when the connection gets opened,
@@ -531,6 +531,9 @@ _Function_class_(QUIC_CONNECTION_CALLBACK)
 		break;
 	case QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE: {
 		CQuicUserContextRelease releaseUser(user); // Hopefully we only get QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE once!
+#ifdef FE_DEBUG_QUIC
+		user->DebugRefCount = true;
+#endif
 		user->MaxSendLength = 0;
 		nlinfo("Shutdown complete");
 		if (ev->SHUTDOWN_COMPLETE.AppCloseInProgress)
diff --git a/ryzom/server/src/frontend_service/quic_transceiver.h b/ryzom/server/src/frontend_service/quic_transceiver.h
index 0c610b159b..a5cf1b7b66 100644
--- a/ryzom/server/src/frontend_service/quic_transceiver.h
+++ b/ryzom/server/src/frontend_service/quic_transceiver.h
@@ -58,6 +58,10 @@ A wizard's code, that's sure to bring you cheer.
 
 */
 
+//#ifdef NL_DEBUG
+#define FE_DEBUG_QUIC
+//#endif
+
 class CClientHost;
 
 class CQuicTransceiverImpl;
@@ -76,13 +80,22 @@ class CQuicUserContext
 	// Manual reference count
 	void increaseRef()
 	{
-		m_RefCount.fetch_add(1, std::memory_order_relaxed);
+		int value = m_RefCount.fetch_add(1, std::memory_order_relaxed) + 1;
+#ifdef FE_DEBUG_QUIC
+		if (DebugRefCount)
+			nldebug("Reference count [%p] is now %i", this, value);
+#endif
 	}
 
 	void decreaseRef()
 	{
+		int value = m_RefCount.fetch_sub(1, std::memory_order_release) - 1;
+#ifdef FE_DEBUG_QUIC
+		if (DebugRefCount)
+			nldebug("Reference count [%p] is now %i", this, value);
+#endif
 		// Release order to ensure this thread is done with this object
-		if (m_RefCount.fetch_sub(1, std::memory_order_release) == 1)
+		if (value == 0)
 		{
 			// Acquire order to ensure all other threads are done with this object
 			std::atomic_thread_fence(std::memory_order_acquire);
@@ -118,6 +131,11 @@ class CQuicUserContext
 	CQuicBuffer SendQuicBuffer;
 	NLMISC::CAtomicInt SentCount;
 
+#ifdef FE_DEBUG_QUIC
+	// After shutdown, enable ref count debugging
+	NLMISC::CAtomicBool DebugRefCount;
+#endif
+
 private:
 	std::atomic_int m_RefCount = 0;
 };
@@ -169,6 +187,16 @@ class CQuicUserContextPtr
 			m_User->increaseRef();
 	}
 
+	CQuicUserContextPtr &operator=(CQuicUserContext *user)
+	{
+		if (m_User)
+			m_User->decreaseRef();
+		m_User = user;
+		if (m_User)
+			m_User->increaseRef();
+		return *this;
+	}
+
 	CQuicUserContextPtr &operator=(const CQuicUserContextPtr &other)
 	{
 		if (m_User)