Skip to content

Commit

Permalink
fix(core): Stop processing after a synchronous response
Browse files Browse the repository at this point in the history
Before it was possible that two responses were received at the same
time. If the client is waiting for the first response, then the second
was processed also before returning the control flow to the client.

This lead to situations where a PublishResponse was processed before
notifying the client about a MonitoredItem handle established just
before.

This fixes open62541#5226.
  • Loading branch information
jpfr committed Nov 17, 2024
1 parent 633e210 commit 155dfb0
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/client/ua_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,11 @@ processMSGResponse(UA_Client *client, UA_UInt32 requestId,
UA_clear(response, ac->responseType);
UA_free(ac);
} else {
/* Return a special status code after processing a synchronous message.
* This makes the client return control immediately. */
ac->syncResponse = NULL; /* Indicate that response was received */
if(retval == UA_STATUSCODE_GOOD)
retval = UA_STATUSCODE_GOODCOMPLETESASYNCHRONOUSLY;
}
return retval;
}
Expand Down
30 changes: 30 additions & 0 deletions src/client/ua_client_connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,9 @@ verifyClientApplicationURI(const UA_Client *client) {
#endif
}

static void
delayedNetworkCallback(void *application, void *context);

static void
__Client_networkCallback(UA_ConnectionManager *cm, uintptr_t connectionId,
void *application, void **connectionContext,
Expand Down Expand Up @@ -1605,6 +1608,21 @@ __Client_networkCallback(UA_ConnectionManager *cm, uintptr_t connectionId,
messageType, requestId, &payload);
if(copied)
UA_ByteString_clear(&payload);

/* Abort after synchronous processing of a message.
* Add a delayed callback to process the remaining buffer ASAP. */
if(res == UA_STATUSCODE_GOODCOMPLETESASYNCHRONOUSLY) {
if(client->channel.unprocessed.length > client->channel.unprocessedOffset &&
client->channel.unprocessedDelayed.callback == NULL) {
client->channel.unprocessedDelayed.callback = delayedNetworkCallback;
client->channel.unprocessedDelayed.application = client;
client->channel.unprocessedDelayed.context = &client->channel;
UA_EventLoop *el = client->config.eventLoop;
el->addDelayedCallback(el, &client->channel.unprocessedDelayed);
}
res = UA_STATUSCODE_GOOD;
break;
}
}
res |= UA_SecureChannel_persistBuffer(&client->channel);

Expand Down Expand Up @@ -1639,6 +1657,18 @@ __Client_networkCallback(UA_ConnectionManager *cm, uintptr_t connectionId,
UA_UNLOCK(&client->clientMutex);
}

static void
delayedNetworkCallback(void *application, void *context) {
UA_Client *client = (UA_Client*)application;
client->channel.unprocessedDelayed.callback = NULL;
if(client->channel.state == UA_SECURECHANNELSTATE_CONNECTED)
__Client_networkCallback(client->channel.connectionManager,
client->channel.connectionId,
client, &context,
UA_CONNECTIONSTATE_ESTABLISHED,
&UA_KEYVALUEMAP_NULL, UA_BYTESTRING_NULL);
}

/* Initialize a TCP connection. Writes the result to client->connectStatus. */
static void
initConnect(UA_Client *client) {
Expand Down
7 changes: 7 additions & 0 deletions src/ua_securechannel.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ UA_SecureChannel_clear(UA_SecureChannel *channel) {
channel->channelContext = NULL;
}

/* Remove remaining delayed callback */
if(channel->connectionManager &&
channel->connectionManager->eventSource.eventLoop) {
UA_EventLoop *el = channel->connectionManager->eventSource.eventLoop;
el->removeDelayedCallback(el, &channel->unprocessedDelayed);
}

/* The EventLoop connection is no longer valid */
channel->connectionId = 0;
channel->connectionManager = NULL;
Expand Down
1 change: 1 addition & 0 deletions src/ua_securechannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ struct UA_SecureChannel {
UA_ByteString unprocessed;
size_t unprocessedOffset;
UA_Boolean unprocessedCopied;
UA_DelayedCallback unprocessedDelayed;

UA_CertificateVerification *certificateVerification;
void *processOPNHeaderApplication;
Expand Down

0 comments on commit 155dfb0

Please sign in to comment.