[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #345: GUACAMOLE-1388: Ensure RDP-specific resources are cleaned up after channel disconnect.

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #345:
URL: https://github.com/apache/guacamole-server/pull/345#discussion_r679553973



##########
File path: src/protocols/rdp/rdp.c
##########
@@ -816,6 +804,18 @@ void* guac_rdp_client_thread(void* data) {
     }
 #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);
+    }
+

Review comment:
       As I walked through the set of resources that are allocated/freed for each (re)connection attempt, I noticed that this particular resource is created and destroyed asymmetrically. It was allocated for each reconnect, but freed only at the absolute end of the connection, thus each reconnect would erroneously start a new recording and fail to properly free resources.
   
   It struck me as related (mishandled resources on reconnect), but you're right that it's not related to channel disconnect and not related to specifically the clipboard issue noted in GUACAMOLE-1388.
   
   Should I separate this into a different commit and broaden the issue+PR description?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org