JSON-RPC: fix connect/disconnect crash, consolidate connection notifications

Three related changes to the client connection RPC introduced by this PR:

1. Crash fix. CClient::Stop() ran the event loop (QCoreApplication::processEvents)
   for its 100 ms settle. Called from the connect/disconnect handlers, that
   re-entered the JSON-RPC readyRead handler while it was still on the stack and
   freed a socket that was then written to: a use-after-free (stack overflow under
   overlapping requests). Keep the settle but do it with QThread::msleep, so no
   events are pumped and no re-entrancy is possible. Verified with AddressSanitizer
   on Linux and macOS: overlapping connect/disconnect plus 8-thread churn crash
   before this change and are clean after; a plain deferral and a re-entrancy guard
   were each insufficient.

2. Consolidate notifications. Fold jamulusclient/connecting and
   jamulusclient/connectingFailed into jamulusclient/connectionStateChanged, which
   now carries serverName and, on a failed attempt, error (serverName is omitted
   there, since an invalid address fails before the name is set). One notification
   follows the whole lifecycle; connected/disconnected are unchanged.

3. Rename jamulusclient/connect to jamulusclient/requestConnection: it returns "ok"
   as soon as the attempt is initiated, not when it succeeds, so the name reflects a
   request. Reads symmetrically with getConnectionState.

docs/JSON-RPC.md regenerated.

CHANGELOG: fix a client crash when connect/disconnect are driven over JSON-RPC

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEyPQdxy6h7vQtUDCh2KY2

Author: jrd

docs/JSON-RPC.md

@@ -129,24 +129,6 @@ Results:
 | result.version | string | The Jamulus version. |
 
 
-### jamulusclient/connect
-
-Connects the client to a server. Any current connection is terminated first. The connection is established asynchronously: subscribe to the jamulusclient/connecting, jamulusclient/connected, jamulusclient/connectingFailed and jamulusclient/connectionStateChanged notifications to follow its progress.
-
-Parameters:
-
-| Name | Type | Description |
-| --- | --- | --- |
-| params.address | string | Socket address of the server (host:port). |
-| params.serverName | string | Optional human readable server name used for display purposes. Defaults to the address. |
-
-Results:
-
-| Name | Type | Description |
-| --- | --- | --- |
-| result | string | "ok" once the connection attempt has been initiated. |
-
-
 ### jamulusclient/disconnect
 
 Disconnects the client from the current server. Does nothing if the client is not connected.
@@ -309,6 +291,24 @@ Results:
 | result | string | "ok" or "error" if bad arguments. |
 
 
+### jamulusclient/requestConnection
+
+Connects the client to a server. Any current connection is terminated first. The connection is established asynchronously: subscribe to the jamulusclient/connected and jamulusclient/connectionStateChanged notifications to follow its progress (a failed attempt arrives as connectionStateChanged with state "disconnected" and an error field).
+
+Parameters:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| params.address | string | Socket address of the server (host:port). |
+| params.serverName | string | Optional human readable server name used for display purposes; if given it must be a string. Defaults to the address. |
+
+Results:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| result | string | "ok" once the connection attempt has been initiated. |
+
+
 ### jamulusclient/sendChatText
 
 Sends a chat text message.
@@ -709,37 +709,17 @@ Parameters:
 | params.id | number | The channel ID assigned to the client. |
 
 
-### jamulusclient/connecting
-
-Emitted when a connection to a server has been requested but is not yet established.
-
-Parameters:
-
-| Name | Type | Description |
-| --- | --- | --- |
-| params.serverName | string | The human readable server name (or the address if no name is known). |
-
-
-### jamulusclient/connectingFailed
-
-Emitted when a connection attempt failed before it could be requested from the server.
-
-Parameters:
-
-| Name | Type | Description |
-| --- | --- | --- |
-| params.error | string | The error message. |
-
-
 ### jamulusclient/connectionStateChanged
 
-Emitted whenever the connection state changes.
+Emitted whenever the connection state changes. On a failed connection attempt it is emitted with state "disconnected" and an additional error field.
 
 Parameters:
 
 | Name | Type | Description |
 | --- | --- | --- |
 | params.state | string | The new connection state (disconnected, connecting, or connected). |
+| params.serverName | string | The human readable server name (empty/absent when disconnected). |
+| params.error | string | Only present on a failed connection attempt (with state "disconnected"); serverName is omitted in that case. |
 
 
 ### jamulusclient/disconnected

src/client.cpp

@@ -45,6 +45,7 @@
 \******************************************************************************/
 
 #include "client.h"
+#include <QThread>
 #include "settings.h"
 #include "util.h"
 
@@ -1108,19 +1109,12 @@ void CClient::Stop()
         qWarning() << "Could not reinitialise the sound device while disconnecting:" << generr.GetErrorText();
     }
 
-    // wait for approx. 100 ms to make sure no audio packet is still in the
-    // network queue causing the channel to be reconnected right after having
-    // received the disconnect message (seems not to gain much, disconnect is
-    // still not working reliably)
-    QTime DieTime = QTime::currentTime().addMSecs ( 100 );
-    while ( QTime::currentTime() < DieTime )
-    {
-        // exclude user input events because if we use AllEvents, it happens
-        // that if the user initiates a connection and disconnection quickly
-        // (e.g. quickly pressing enter five times), the software can get into
-        // an unknown state
-        QCoreApplication::processEvents ( QEventLoop::ExcludeUserInputEvents, 100 );
-    }
+    // Wait ~100 ms so no audio packet is still in the network queue causing the
+    // channel to be reconnected right after the disconnect message. We must NOT
+    // run the event loop to do this: pumping events here re-entered the JSON-RPC
+    // read handler and freed a socket still being written to (use-after-free). A
+    // plain sleep keeps the settle without re-entrancy.
+    QThread::msleep ( 100 );
 
     // Send disconnect message to server (Since we disable our protocol
     // receive mechanism with the next command, we do not evaluate any

src/clientrpc.cpp

@@ -183,33 +183,28 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
     /// @param {object} params - No parameters (empty object).
     connect ( pClient, &CClient::Disconnected, [=]() { pRpcServer->BroadcastNotification ( "jamulusclient/disconnected", QJsonObject{} ); } );
 
-    /// @rpc_notification jamulusclient/connecting
-    /// @brief Emitted when a connection to a server has been requested but is not yet established.
-    /// @param {string} params.serverName - The human readable server name (or the address if no name is known).
-    connect ( pClient, &CClient::Connecting, [=] ( QString strServerName ) {
-        pRpcServer->BroadcastNotification ( "jamulusclient/connecting",
-                                            QJsonObject{
-                                                { "serverName", strServerName },
-                                            } );
-    } );
-
-    /// @rpc_notification jamulusclient/connectingFailed
-    /// @brief Emitted when a connection attempt failed before it could be requested from the server.
-    /// @param {string} params.error - The error message.
+    // A failed attempt surfaces through connectionStateChanged with the error attached.
+    // serverName is omitted here: on the invalid-address path the attempt throws before the
+    // name is set, so it is not reliably known.
     connect ( pClient, &CClient::ConnectingFailed, [=] ( QString strError ) {
-        pRpcServer->BroadcastNotification ( "jamulusclient/connectingFailed",
+        pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
                                             QJsonObject{
+                                                { "state", ConnectionStateToString ( CS_DISCONNECTED ) },
                                                 { "error", strError },
                                             } );
     } );
 
     /// @rpc_notification jamulusclient/connectionStateChanged
-    /// @brief Emitted whenever the connection state changes.
+    /// @brief Emitted whenever the connection state changes. On a failed connection attempt it is
+    ///        emitted with state "disconnected" and an additional error field.
     /// @param {string} params.state - The new connection state (disconnected, connecting, or connected).
+    /// @param {string} params.serverName - The human readable server name (empty/absent when disconnected).
+    /// @param {string} params.error - Only present on a failed connection attempt (with state "disconnected"); serverName is omitted in that case.
     connect ( pClient, &CClient::ConnectionStateChanged, [=] ( EConnectionState eState ) {
         pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
                                             QJsonObject{
                                                 { "state", ConnectionStateToString ( eState ) },
+                                                { "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
                                             } );
     } );
 
@@ -257,25 +252,31 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
         Q_UNUSED ( params );
     } );
 
-    /// @rpc_method jamulusclient/connect
+    /// @rpc_method jamulusclient/requestConnection
     /// @brief Connects the client to a server. Any current connection is terminated first.
-    ///        The connection is established asynchronously: subscribe to the jamulusclient/connecting,
-    ///        jamulusclient/connected, jamulusclient/connectingFailed and jamulusclient/connectionStateChanged
-    ///        notifications to follow its progress.
+    ///        The connection is established asynchronously: subscribe to the jamulusclient/connected
+    ///        and jamulusclient/connectionStateChanged notifications to follow its progress (a failed
+    ///        attempt arrives as connectionStateChanged with state "disconnected" and an error field).
     /// @param {string} params.address - Socket address of the server (host:port).
-    /// @param {string} params.serverName - Optional human readable server name used for display purposes. Defaults to the address.
+    /// @param {string} params.serverName - Optional human readable server name used for display purposes; if given it must be a string. Defaults to the address.
     /// @result {string} result - "ok" once the connection attempt has been initiated.
-    pRpcServer->HandleMethod ( "jamulusclient/connect", [=] ( const QJsonObject& params, QJsonObject& response ) {
+    pRpcServer->HandleMethod ( "jamulusclient/requestConnection", [=] ( const QJsonObject& params, QJsonObject& response ) {
         auto jsonAddress = params["address"];
         if ( !jsonAddress.isString() )
         {
             response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a string" );
             return;
         }
 
-        auto          jsonServerName = params["serverName"];
-        const QString strAddress     = NetworkUtil::FixAddress ( jsonAddress.toString() );
-        const QString strServerName  = jsonServerName.isString() ? jsonServerName.toString() : strAddress;
+        auto jsonServerName = params["serverName"];
+        if ( !jsonServerName.isUndefined() && !jsonServerName.isNull() && !jsonServerName.isString() )
+        {
+            response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: serverName is not a string" );
+            return;
+        }
+
+        const QString strAddress    = NetworkUtil::FixAddress ( jsonAddress.toString() );
+        const QString strServerName = jsonServerName.isString() ? jsonServerName.toString() : strAddress;
 
         pClient->Connect ( strAddress, strServerName );