GUACAMOLE-1388: Ensure RDP-specific resources are cleaned up after channel disconnect.

Without these changes, RDP-specific resources like the CLIPRDR and RDPEI
channels may remain from past connections if the RDP connection is
dynamically reconnected via the "Reconnect" display resize method,
resulting in assertion failures or memory errors if those stale
resources are reused after reconnect is completed.
This commit is contained in:
Michael Jumper 2021-07-28 15:50:13 -07:00
parent 91d79da49f
commit 2524af80a9
4 changed files with 137 additions and 12 deletions

View File

@ -546,6 +546,48 @@ static void guac_rdp_cliprdr_channel_connected(rdpContext* context,
} }
/**
* Callback which disassociates Guacamole from the CliprdrClientContext
* instance that was originally allocated by FreeRDP and is about to be
* deallocated.
*
* This function is called whenever a channel disconnects via the PubSub event
* system within FreeRDP, but only has any effect if the disconnected channel
* is the CLIPRDR channel. This specific callback is registered with the PubSub
* system of the relevant rdpContext when guac_rdp_clipboard_load_plugin() is
* called.
*
* @param context
* The rdpContext associated with the active RDP session.
*
* @param e
* Event-specific arguments, mainly the name of the channel, and a
* reference to the associated plugin loaded for that channel by FreeRDP.
*/
static void guac_rdp_cliprdr_channel_disconnected(rdpContext* context,
ChannelDisconnectedEventArgs* e) {
guac_client* client = ((rdp_freerdp_context*) context)->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
guac_rdp_clipboard* clipboard = rdp_client->clipboard;
/* FreeRDP-specific handlers for CLIPRDR are not assigned, and thus not
* callable, until after the relevant guac_rdp_clipboard structure is
* allocated and associated with the guac_rdp_client */
assert(clipboard != NULL);
/* Ignore disconnection event if it's not for the CLIPRDR channel */
if (strcmp(e->name, CLIPRDR_SVC_CHANNEL_NAME) != 0)
return;
/* Channel is no longer connected */
clipboard->cliprdr = NULL;
guac_client_log(client, GUAC_LOG_DEBUG, "CLIPRDR (clipboard redirection) "
"channel disconnected.");
}
guac_rdp_clipboard* guac_rdp_clipboard_alloc(guac_client* client) { guac_rdp_clipboard* guac_rdp_clipboard_alloc(guac_client* client) {
/* Allocate clipboard and underlying storage */ /* Allocate clipboard and underlying storage */
@ -575,6 +617,10 @@ void guac_rdp_clipboard_load_plugin(guac_rdp_clipboard* clipboard,
PubSub_SubscribeChannelConnected(context->pubSub, PubSub_SubscribeChannelConnected(context->pubSub,
(pChannelConnectedEventHandler) guac_rdp_cliprdr_channel_connected); (pChannelConnectedEventHandler) guac_rdp_cliprdr_channel_connected);
/* Clean up any RDP-specific resources when channel is disconnected */
PubSub_SubscribeChannelDisconnected(context->pubSub,
(pChannelDisconnectedEventHandler) guac_rdp_cliprdr_channel_disconnected);
guac_client_log(clipboard->client, GUAC_LOG_DEBUG, "Support for CLIPRDR " guac_client_log(clipboard->client, GUAC_LOG_DEBUG, "Support for CLIPRDR "
"(clipboard redirection) registered. Awaiting channel " "(clipboard redirection) registered. Awaiting channel "
"connection."); "connection.");

View File

@ -96,12 +96,52 @@ static void guac_rdp_disp_channel_connected(rdpContext* context,
} }
/**
* Callback which disassociates Guacamole from the DispClientContext instance
* that was originally allocated by FreeRDP and is about to be deallocated.
*
* This function is called whenever a channel disconnects via the PubSub event
* system within FreeRDP, but only has any effect if the disconnected channel
* is the Display Update channel. This specific callback is registered with the
* PubSub system of the relevant rdpContext when guac_rdp_disp_load_plugin() is
* called.
*
* @param context
* The rdpContext associated with the active RDP session.
*
* @param e
* Event-specific arguments, mainly the name of the channel, and a
* reference to the associated plugin loaded for that channel by FreeRDP.
*/
static void guac_rdp_disp_channel_disconnected(rdpContext* context,
ChannelDisconnectedEventArgs* e) {
guac_client* client = ((rdp_freerdp_context*) context)->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
guac_rdp_disp* guac_disp = rdp_client->disp;
/* Ignore disconnection event if it's not for the Display Update channel */
if (strcmp(e->name, DISP_DVC_CHANNEL_NAME) != 0)
return;
/* Channel is no longer connected */
guac_disp->disp = NULL;
guac_client_log(client, GUAC_LOG_DEBUG, "Display update channel "
"disconnected.");
}
void guac_rdp_disp_load_plugin(rdpContext* context) { void guac_rdp_disp_load_plugin(rdpContext* context) {
/* Subscribe to and handle channel connected events */ /* Subscribe to and handle channel connected events */
PubSub_SubscribeChannelConnected(context->pubSub, PubSub_SubscribeChannelConnected(context->pubSub,
(pChannelConnectedEventHandler) guac_rdp_disp_channel_connected); (pChannelConnectedEventHandler) guac_rdp_disp_channel_connected);
/* Subscribe to and handle channel disconnected events */
PubSub_SubscribeChannelDisconnected(context->pubSub,
(pChannelDisconnectedEventHandler) guac_rdp_disp_channel_disconnected);
/* Add "disp" channel */ /* Add "disp" channel */
guac_freerdp_dynamic_channel_collection_add(context->settings, "disp", NULL); guac_freerdp_dynamic_channel_collection_add(context->settings, "disp", NULL);

View File

@ -94,12 +94,51 @@ static void guac_rdp_rdpei_channel_connected(rdpContext* context,
} }
/**
* Callback which disassociates Guacamole from the RdpeiClientContext instance
* that was originally allocated by FreeRDP and is about to be deallocated.
*
* This function is called whenever a channel disconnects via the PubSub event
* system within FreeRDP, but only has any effect if the disconnected channel
* is the RDPEI channel. This specific callback is registered with the PubSub
* system of the relevant rdpContext when guac_rdp_rdpei_load_plugin() is
* called.
*
* @param context
* The rdpContext associated with the active RDP session.
*
* @param e
* Event-specific arguments, mainly the name of the channel, and a
* reference to the associated plugin loaded for that channel by FreeRDP.
*/
static void guac_rdp_rdpei_channel_disconnected(rdpContext* context,
ChannelDisconnectedEventArgs* e) {
guac_client* client = ((rdp_freerdp_context*) context)->client;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
guac_rdp_rdpei* guac_rdpei = rdp_client->rdpei;
/* Ignore disconnection event if it's not for the RDPEI channel */
if (strcmp(e->name, RDPEI_DVC_CHANNEL_NAME) != 0)
return;
/* Channel is no longer connected */
guac_rdpei->rdpei = NULL;
guac_client_log(client, GUAC_LOG_DEBUG, "RDPDI channel disconnected.");
}
void guac_rdp_rdpei_load_plugin(rdpContext* context) { void guac_rdp_rdpei_load_plugin(rdpContext* context) {
/* Subscribe to and handle channel connected events */ /* Subscribe to and handle channel connected events */
PubSub_SubscribeChannelConnected(context->pubSub, PubSub_SubscribeChannelConnected(context->pubSub,
(pChannelConnectedEventHandler) guac_rdp_rdpei_channel_connected); (pChannelConnectedEventHandler) guac_rdp_rdpei_channel_connected);
/* Subscribe to and handle channel disconnected events */
PubSub_SubscribeChannelDisconnected(context->pubSub,
(pChannelDisconnectedEventHandler) guac_rdp_rdpei_channel_disconnected);
/* Add "rdpei" channel */ /* Add "rdpei" channel */
guac_freerdp_dynamic_channel_collection_add(context->settings, "rdpei", NULL); guac_freerdp_dynamic_channel_collection_add(context->settings, "rdpei", NULL);

View File

@ -471,18 +471,6 @@ static int guac_rdp_handle_connection(guac_client* client) {
pthread_rwlock_wrlock(&(rdp_client->lock)); pthread_rwlock_wrlock(&(rdp_client->lock));
/* Set up screen recording, if requested */
if (settings->recording_path != NULL) {
rdp_client->recording = guac_common_recording_create(client,
settings->recording_path,
settings->recording_name,
settings->create_recording_path,
!settings->recording_exclude_output,
!settings->recording_exclude_mouse,
!settings->recording_exclude_touch,
settings->recording_include_keys);
}
/* Create display */ /* Create display */
rdp_client->display = guac_common_display_alloc(client, rdp_client->display = guac_common_display_alloc(client,
rdp_client->settings->width, rdp_client->settings->width,
@ -816,6 +804,18 @@ void* guac_rdp_client_thread(void* data) {
} }
#endif #endif
/* Set up screen recording, if requested */
if (settings->recording_path != NULL) {
rdp_client->recording = guac_common_recording_create(client,
settings->recording_path,
settings->recording_name,
settings->create_recording_path,
!settings->recording_exclude_output,
!settings->recording_exclude_mouse,
!settings->recording_exclude_touch,
settings->recording_include_keys);
}
/* Continue handling connections until error or client disconnect */ /* Continue handling connections until error or client disconnect */
while (client->state == GUAC_CLIENT_RUNNING) { while (client->state == GUAC_CLIENT_RUNNING) {
if (guac_rdp_handle_connection(client)) if (guac_rdp_handle_connection(client))