Skip to content

Commit a75cd19

Browse files
jrdclaude
andcommitted
Add JSON-RPC client connection control
New methods: jamulusclient/requestConnection, jamulusclient/disconnect and jamulusclient/getConnectionState. New notification: jamulusclient/connectionStateChanged, carrying state and serverName and, on a failed attempt, error. Together with the existing connected/disconnected notifications this gives JSON-RPC full parity with the UI for joining and leaving servers (#3801) and makes a --nogui client fully scriptable. requestConnection returns "ok" as soon as the attempt is initiated, not when it succeeds, so the name reflects a request; it reads symmetrically with getConnectionState. An address that cannot be resolved is answered with an invalid-params error and leaves the current connection untouched: the address is resolved before CClient::Connect() disconnects, and the resolved CHostAddress is handed to a new Connect() overload that runs the lifecycle. The GUI and the startup path go through the string overload and get the same protection. NetworkUtil::ParseNetworkAddress reports an SRV record whose target is "." as invalid instead of handing back a null address. SetServerAddr() had no caller left and is removed. 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). The settle is kept but done 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. Measured over JSON-RPC on the headless client: a bad address while connected returns -32602 and the session stays up; a bad address while disconnected emits no notification. docs/JSON-RPC.md regenerated. CHANGELOG: fix a client crash when connect/disconnect are driven over JSON-RPC Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEyPQdxy6h7vQtUDCh2KY2
1 parent c862872 commit a75cd19

5 files changed

Lines changed: 205 additions & 45 deletions

File tree

docs/JSON-RPC.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,23 @@ Results:
129129
| result.version | string | The Jamulus version. |
130130

131131

132+
### jamulusclient/disconnect
133+
134+
Disconnects the client from the current server. Does nothing if the client is not connected.
135+
136+
Parameters:
137+
138+
| Name | Type | Description |
139+
| --- | --- | --- |
140+
| params | object | No parameters (empty object). |
141+
142+
Results:
143+
144+
| Name | Type | Description |
145+
| --- | --- | --- |
146+
| result | string | Always "ok". |
147+
148+
132149
### jamulusclient/getChannelInfo
133150

134151
Returns the client's profile information.
@@ -188,6 +205,24 @@ Results:
188205
| result.clients | array | The client list. See jamulusclient/clientListReceived for the format. |
189206

190207

208+
### jamulusclient/getConnectionState
209+
210+
Returns the current connection state.
211+
212+
Parameters:
213+
214+
| Name | Type | Description |
215+
| --- | --- | --- |
216+
| params | object | No parameters (empty object). |
217+
218+
Results:
219+
220+
| Name | Type | Description |
221+
| --- | --- | --- |
222+
| result.state | string | The connection state (disconnected, connecting, or connected). |
223+
| result.serverName | string | The human readable name of the current server (empty if disconnected). |
224+
225+
191226
### jamulusclient/getCurrentDirectory
192227

193228
Returns the currently selected directory socket address.
@@ -256,6 +291,24 @@ Results:
256291
| result | string | "ok" or "error" if bad arguments. |
257292

258293

294+
### jamulusclient/requestConnection
295+
296+
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). An address that cannot be resolved is rejected with an error and leaves the current connection untouched.
297+
298+
Parameters:
299+
300+
| Name | Type | Description |
301+
| --- | --- | --- |
302+
| params.address | string | Socket address of the server (host:port). |
303+
| params.serverName | string | Optional human readable server name used for display purposes; if given it must be a string (null counts as omitted). Defaults to the address. |
304+
305+
Results:
306+
307+
| Name | Type | Description |
308+
| --- | --- | --- |
309+
| result | string | "ok" once the connection attempt has been initiated. |
310+
311+
259312
### jamulusclient/sendChatText
260313

261314
Sends a chat text message.
@@ -656,6 +709,19 @@ Parameters:
656709
| params.id | number | The channel ID assigned to the client. |
657710

658711

712+
### jamulusclient/connectionStateChanged
713+
714+
Emitted whenever the connection state changes. On a failed connection attempt it is emitted with state "disconnected" and an additional error field.
715+
716+
Parameters:
717+
718+
| Name | Type | Description |
719+
| --- | --- | --- |
720+
| params.state | string | The new connection state (disconnected, connecting, or connected). |
721+
| params.serverName | string | The human readable server name (empty/absent when disconnected). |
722+
| params.error | string | Only present on a failed connection attempt (with state "disconnected"); serverName is omitted in that case. |
723+
724+
659725
### jamulusclient/disconnected
660726

661727
Emitted when the client is disconnected from the server.

src/client.cpp

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
\******************************************************************************/
4646

4747
#include "client.h"
48+
#include <QThread>
4849
#include "settings.h"
4950
#include "util.h"
5051

@@ -619,25 +620,6 @@ void CClient::SetRemoteChanPan ( const int iId, const float fPan )
619620
StartTimerGainOrPan();
620621
}
621622

622-
bool CClient::SetServerAddr ( QString strNAddr )
623-
{
624-
CHostAddress HostAddress;
625-
if ( NetworkUtil::ParseNetworkAddress ( strNAddr, HostAddress, bIPv6Available ) )
626-
{
627-
// apply address to the channel
628-
Channel.SetAddress ( HostAddress );
629-
630-
// By default, set server name to HostAddress. If using the Connect() method, this may be overwritten
631-
SetConnectedServerName ( HostAddress.toString() );
632-
633-
return true;
634-
}
635-
else
636-
{
637-
return false; // invalid address
638-
}
639-
}
640-
641623
bool CClient::GetAndResetbJitterBufferOKFlag()
642624
{
643625
// get the socket buffer put status flag and reset it
@@ -1108,19 +1090,12 @@ void CClient::Stop()
11081090
qWarning() << "Could not reinitialise the sound device while disconnecting:" << generr.GetErrorText();
11091091
}
11101092

1111-
// wait for approx. 100 ms to make sure no audio packet is still in the
1112-
// network queue causing the channel to be reconnected right after having
1113-
// received the disconnect message (seems not to gain much, disconnect is
1114-
// still not working reliably)
1115-
QTime DieTime = QTime::currentTime().addMSecs ( 100 );
1116-
while ( QTime::currentTime() < DieTime )
1117-
{
1118-
// exclude user input events because if we use AllEvents, it happens
1119-
// that if the user initiates a connection and disconnection quickly
1120-
// (e.g. quickly pressing enter five times), the software can get into
1121-
// an unknown state
1122-
QCoreApplication::processEvents ( QEventLoop::ExcludeUserInputEvents, 100 );
1123-
}
1093+
// Wait ~100 ms so no audio packet is still in the network queue causing the
1094+
// channel to be reconnected right after the disconnect message. We must NOT
1095+
// run the event loop to do this: pumping events here re-entered the JSON-RPC
1096+
// read handler and freed a socket still being written to (use-after-free). A
1097+
// plain sleep keeps the settle without re-entrancy.
1098+
QThread::msleep ( 100 );
11241099

11251100
// Send disconnect message to server (Since we disable our protocol
11261101
// receive mechanism with the next command, we do not evaluate any
@@ -1162,30 +1137,38 @@ void CClient::Disconnect()
11621137
/// @method
11631138
/// @brief Connects to strServerAddress. If a connection is currently requested
11641139
/// or established, that connection is terminated first.
1165-
/// @emit Connecting (strServerName) if SetServerAddr was valid. emit happens through Start().
1140+
/// @emit Connecting (strServerName) if the address resolved. emit happens through Start().
11661141
/// Use to set CClientDlg to show being connected
11671142
/// @emit ConnectingFailed (error) if an error occurred
11681143
/// Use to display error message in CClientDlg
11691144
/// @param strServerAddress - the server address to connect to
11701145
/// @param strServerName - the human readable server name passed to Connecting()
11711146
void CClient::Connect ( const QString& strServerAddress, const QString& strServerName )
1147+
{
1148+
// resolve before touching the current connection, so that an invalid
1149+
// address leaves it in place
1150+
CHostAddress HostAddress;
1151+
1152+
if ( !NetworkUtil::ParseNetworkAddress ( strServerAddress, HostAddress, bIPv6Available ) )
1153+
{
1154+
emit ConnectingFailed ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) );
1155+
return;
1156+
}
1157+
1158+
Connect ( HostAddress, strServerName );
1159+
}
1160+
1161+
void CClient::Connect ( const CHostAddress& HostAddress, const QString& strServerName )
11721162
{
11731163
try
11741164
{
11751165
// disconnect from any current server first so that connecting to a
11761166
// different server while connected behaves as a reconnect
11771167
Disconnect();
11781168

1179-
// Set server address and connect if valid address was supplied
1180-
if ( SetServerAddr ( strServerAddress ) )
1181-
{
1182-
SetConnectedServerName ( strServerName );
1183-
Start();
1184-
}
1185-
else
1186-
{
1187-
throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) );
1188-
}
1169+
Channel.SetAddress ( HostAddress );
1170+
SetConnectedServerName ( strServerName );
1171+
Start();
11891172
}
11901173
catch ( const CGenErr& generr )
11911174
{

src/client.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ class CClient : public QObject
171171

172172
void Disconnect();
173173
void Connect ( const QString& strServerAddress, const QString& strServerName );
174+
void Connect ( const CHostAddress& HostAddress, const QString& strServerName );
174175

175176
// The ConnectedServerName is emitted by Connecting() to update the UI with a human readable server name
176177
void SetConnectedServerName ( const QString& strServerName ) { strConnectedServerName = strServerName; };
@@ -180,7 +181,6 @@ class CClient : public QObject
180181

181182
bool IsRunning() { return Sound.IsRunning(); }
182183
bool IsCallbackEntered() const { return Sound.IsCallbackEntered(); }
183-
bool SetServerAddr ( QString strNAddr );
184184

185185
// IPv6 Available
186186
bool IsIPv6Available() { return bIPv6Available; }

src/clientrpc.cpp

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@
4747

4848
#include "clientrpc.h"
4949

50+
static QString ConnectionStateToString ( const EConnectionState eState )
51+
{
52+
switch ( eState )
53+
{
54+
case CS_CONNECTING:
55+
return "connecting";
56+
57+
case CS_CONNECTED:
58+
return "connected";
59+
60+
default:
61+
return "disconnected";
62+
}
63+
}
64+
5065
CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServer* pRpcServer, QObject* parent ) :
5166
QObject ( parent ),
5267
m_pSettings ( pSettings )
@@ -168,6 +183,30 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
168183
/// @param {object} params - No parameters (empty object).
169184
connect ( pClient, &CClient::Disconnected, [=]() { pRpcServer->BroadcastNotification ( "jamulusclient/disconnected", QJsonObject{} ); } );
170185

186+
// A failed attempt surfaces through connectionStateChanged with the error attached.
187+
// serverName is omitted here: an invalid address fails before the name is set.
188+
connect ( pClient, &CClient::ConnectingFailed, [=] ( QString strError ) {
189+
pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
190+
QJsonObject{
191+
{ "state", ConnectionStateToString ( CS_DISCONNECTED ) },
192+
{ "error", strError },
193+
} );
194+
} );
195+
196+
/// @rpc_notification jamulusclient/connectionStateChanged
197+
/// @brief Emitted whenever the connection state changes. On a failed connection attempt it is
198+
/// emitted with state "disconnected" and an additional error field.
199+
/// @param {string} params.state - The new connection state (disconnected, connecting, or connected).
200+
/// @param {string} params.serverName - The human readable server name (empty/absent when disconnected).
201+
/// @param {string} params.error - Only present on a failed connection attempt (with state "disconnected"); serverName is omitted in that case.
202+
connect ( pClient, &CClient::ConnectionStateChanged, [=] ( EConnectionState eState ) {
203+
pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
204+
QJsonObject{
205+
{ "state", ConnectionStateToString ( eState ) },
206+
{ "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
207+
} );
208+
} );
209+
171210
/// @rpc_notification jamulusclient/recorderState
172211
/// @brief Emitted when the client is connected to a server whose recorder state changes.
173212
/// @param {number} params.state - The recorder state.
@@ -212,6 +251,76 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
212251
Q_UNUSED ( params );
213252
} );
214253

254+
/// @rpc_method jamulusclient/requestConnection
255+
/// @brief Connects the client to a server. Any current connection is terminated first.
256+
/// The connection is established asynchronously: subscribe to the jamulusclient/connected
257+
/// and jamulusclient/connectionStateChanged notifications to follow its progress (a failed
258+
/// attempt arrives as connectionStateChanged with state "disconnected" and an error field).
259+
/// An address that cannot be resolved is rejected with an error and leaves the current
260+
/// connection untouched.
261+
/// @param {string} params.address - Socket address of the server (host:port).
262+
/// @param {string} params.serverName - Optional human readable server name used for display purposes; if given it must be a string
263+
/// (null counts as omitted). Defaults to the address.
264+
/// @result {string} result - "ok" once the connection attempt has been initiated.
265+
pRpcServer->HandleMethod ( "jamulusclient/requestConnection", [=] ( const QJsonObject& params, QJsonObject& response ) {
266+
auto jsonAddress = params["address"];
267+
if ( !jsonAddress.isString() )
268+
{
269+
response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a string" );
270+
return;
271+
}
272+
273+
auto jsonServerName = params["serverName"];
274+
if ( !jsonServerName.isUndefined() && !jsonServerName.isNull() && !jsonServerName.isString() )
275+
{
276+
response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: serverName is not a string" );
277+
return;
278+
}
279+
280+
const QString strAddress = NetworkUtil::FixAddress ( jsonAddress.toString() );
281+
const QString strServerName = jsonServerName.isString() ? jsonServerName.toString() : strAddress;
282+
283+
// resolve here so that the caller gets an error result for an invalid address
284+
CHostAddress haServer;
285+
if ( !NetworkUtil::ParseNetworkAddress ( strAddress, haServer, pClient->IsIPv6Available() ) )
286+
{
287+
response["error"] =
288+
CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a valid socket address" );
289+
return;
290+
}
291+
292+
pClient->Connect ( haServer, strServerName );
293+
294+
response["result"] = "ok";
295+
} );
296+
297+
/// @rpc_method jamulusclient/disconnect
298+
/// @brief Disconnects the client from the current server. Does nothing if the client is not connected.
299+
/// @param {object} params - No parameters (empty object).
300+
/// @result {string} result - Always "ok".
301+
pRpcServer->HandleMethod ( "jamulusclient/disconnect", [=] ( const QJsonObject& params, QJsonObject& response ) {
302+
pClient->Disconnect();
303+
304+
response["result"] = "ok";
305+
Q_UNUSED ( params );
306+
} );
307+
308+
/// @rpc_method jamulusclient/getConnectionState
309+
/// @brief Returns the current connection state.
310+
/// @param {object} params - No parameters (empty object).
311+
/// @result {string} result.state - The connection state (disconnected, connecting, or connected).
312+
/// @result {string} result.serverName - The human readable name of the current server (empty if disconnected).
313+
pRpcServer->HandleMethod ( "jamulusclient/getConnectionState", [=] ( const QJsonObject& params, QJsonObject& response ) {
314+
const EConnectionState eState = pClient->GetConnectionState();
315+
316+
QJsonObject result{
317+
{ "state", ConnectionStateToString ( eState ) },
318+
{ "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
319+
};
320+
response["result"] = result;
321+
Q_UNUSED ( params );
322+
} );
323+
215324
/// @rpc_method jamulus/getMode
216325
/// @brief Returns the current mode, i.e. whether Jamulus is running as a server or client.
217326
/// @param {object} params - No parameters (empty object).

src/util.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,9 @@ bool NetworkUtil::ParseNetworkAddress ( QString strAddress, CHostAddress& HostAd
944944
// Try SRV-based discovery first:
945945
if ( ParseNetworkAddressSrv ( strAddress, HostAddress, bIPv6Available ) )
946946
{
947-
return true;
947+
// an SRV target of "." means the service is not offered: fail here
948+
// rather than falling back to a host lookup
949+
return !HostAddress.InetAddr.isNull();
948950
}
949951
#endif
950952
// Try regular connect via plain IP or host name lookup (A/AAAA):

0 commit comments

Comments
 (0)