Handle connection shutdown and close, ref #628

ryzom/server/src/frontend_service/fe_receive_sub.cpp

@@ -642,6 +642,7 @@ void CFeReceiveSub::removeClientFromMap( CClientHost *client )
 		{
 			client->QuicUser->ClientHost = nullptr;
 		}
+		client->QuicUser->Transceiver->shutdown(client->QuicUser.get());
 		client->QuicUser = nullptr;
 	}
 }

ryzom/server/src/frontend_service/quic_transceiver.cpp

@@ -333,6 +333,23 @@ void CQuicTransceiver::release()
 	}
 }
 
+CQuicUserContext::CQuicUserContext()
+{
+}
+
+CQuicUserContext::~CQuicUserContext()
+{
+	// This should never get called before the connection is shutdown,
+	// since we increase the reference when the connection gets opened,
+	// and decrease the reference when the connection is shutdown.
+	CQuicTransceiverImpl *m = Transceiver->m.get();
+	if (Connection) // Should always be set, this is already warned for in the shutdown handling.
+	{
+		MsQuic->ConnectionClose((HQUIC)Connection);
+		Connection = null;
+	}
+}
+
 QUIC_STATUS
 #ifdef _Function_class_
 _Function_class_(QUIC_LISTENER_CALLBACK)
@@ -389,6 +406,7 @@ _Function_class_(QUIC_CONNECTION_CALLBACK)
 		nlinfo("Connected");
 		nlassert(CStringView((const char *)ev->CONNECTED.NegotiatedAlpn, ev->CONNECTED.NegotiatedAlpnLength) == "ryzomcore4");
 		MsQuic->ConnectionSendResumptionTicket(connection, QUIC_SEND_RESUMPTION_FLAG_NONE, 0, NULL); // What does this even do?
+		status = QUIC_STATUS_SUCCESS;
 		break;
 	case QUIC_CONNECTION_EVENT_SHUTDOWN_INITIATED_BY_TRANSPORT:
 		nlinfo("Shutdown initiated by transport");
@@ -401,23 +419,28 @@ _Function_class_(QUIC_CONNECTION_CALLBACK)
 	case QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE: {
 		CQuicUserContextRelease releaseUser(user); // Hopefully we only get QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE once!
 		nlinfo("Shutdown complete");
-		nlassert(!ev->SHUTDOWN_COMPLETE.AppCloseInProgress); // Only applicable on client, but assert to be sure
-		if (!ev->SHUTDOWN_COMPLETE.AppCloseInProgress)
+		if (ev->SHUTDOWN_COMPLETE.AppCloseInProgress)
 		{
-			MsQuic->ConnectionClose(connection);
+			// Only applicable on client when ConnectionClose is called on the connection, so warn if this happens
+			nlwarning("App close in progress flagged, connection was deleted before the context. This should never happen.");
+			// Should never happen, but just in case, set to null to avoid double deletion
+			user->Connection = null;
 		}
-		// TODO: Report to the boss
+		// TODO: Report to the boss (forge a disconnect datagram from this user)
+		// It's detected through the datagram handling already, anyway
 		status = QUIC_STATUS_SUCCESS;
 		break;
 	}
 	case QUIC_CONNECTION_EVENT_DATAGRAM_RECEIVED:
 		nlinfo("Datagram received");
 		// YES PLEASE
 		self->datagramReceived(user, ev->DATAGRAM_RECEIVED.Buffer->Buffer, ev->DATAGRAM_RECEIVED.Buffer->Length);
+		status = QUIC_STATUS_SUCCESS;
 		break;
 	case QUIC_CONNECTION_EVENT_DATAGRAM_STATE_CHANGED:
 		nlinfo("Datagram state changed");
 		user->MaxSendLength.store(ev->DATAGRAM_STATE_CHANGED.SendEnabled ? ev->DATAGRAM_STATE_CHANGED.MaxSendLength : 0, std::memory_order_release);
+		status = QUIC_STATUS_SUCCESS;
 		break;
 	case QUIC_CONNECTION_EVENT_LOCAL_ADDRESS_CHANGED:
 	case QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED:
@@ -497,8 +520,19 @@ void CQuicTransceiver::sendDatagram(CQuicUserContext *user, const uint8 *buffer,
 	}
 }
 
+void CQuicTransceiver::shutdown(CQuicUserContext *user)
+{
+	// Gracefully shuts down a connection (assume this is called from the main service thread only)
+	// Assuming we can call shutdown as many times as we like...
+	CQuicTransceiverImpl *m = user->Transceiver->m.get();
+	MsQuic->ConnectionShutdown((HQUIC)user->Connection, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0);
+}
+
 #else
 
+using namespace NLMISC;
+using namespace NLNET;
+
 class CQuicTransceiverImpl
 {
 public:
@@ -552,6 +586,10 @@ void CQuicTransceiver::sendDatagram(CQuicUserContext *user, const uint8 *buffer,
 {
 }
 
+void CQuicTransceiver::shutdown(CQuicUserContext *user)
+{
+}
+
 #endif
 
 /* end of file */

ryzom/server/src/frontend_service/quic_transceiver.h

@@ -47,6 +47,9 @@ class CQuicUserContext
 	}
 
 public:
+	CQuicUserContext();
+	~CQuicUserContext();
+
 	// Reference to QUIC context (immutable)
 	CQuicTransceiver *Transceiver;
 
@@ -164,8 +167,12 @@ class CQuicTransceiver
 	/// Send a datagram, fancier than a telegram, but not as reliable
 	void sendDatagram(CQuicUserContext *user, const uint8 *buffer, uint32 size);
 
+	/// Shutdown a connection
+	void shutdown(CQuicUserContext *user);
+
 private:
 	friend CQuicTransceiverImpl;
+	friend CQuicUserContext;
 
 	/// Internal implementation specific
 	std::unique_ptr<CQuicTransceiverImpl> m;