GNOME.org
 

[gnome-remote-desktop] rdp: Get rid of stop_event handling during initialization of DVCs

commit d570f494692abe93165c11aff157b4904664a406
Author: Pascal Nowack <Pascal Nowack gmx de>
Date:   Wed Aug 3 07:19:27 2022 +0200

    rdp: Get rid of stop_event handling during initialization of DVCs
    
    Recently, it was discovered, that WinPR events are handled in
    unexpected manners on a few systems.
    To avoid unexpected behaviour, reduce their usage, wherever it is
    possible.
    
    In order to get rid of the stop event during initialization of DVCs,
    introduce a boolean value, which indicates, whether the session should
    stop.
    This boolean is only set once to TRUE in a session and is used in the
    socket thread to determine, whether a DVC should be initialized or not.
    This behaviour is still necessary, as the graphics pipeline is
    essential for the session.
    If support for the graphics pipeline is advertised, but actually not
    supported by the client, it is an error in the client and the session
    must be stopped.
    
    When stopping all remote desktop sessions via the gnome-shell submenu,
    the RDP backend needs to set the error info to prevent clients from
    doing an autoreconnect.
    Previously, the stop_event was used here too to determine, whether a
    session was stopped via the gnome-shell submenu or not.
    Replace this behaviour by checking now, whether an idle session close
    source was created.

 src/grd-rdp-audio-playback.c    |  6 ------
 src/grd-rdp-audio-playback.h    |  1 -
 src/grd-rdp-display-control.c   |  6 ------
 src/grd-rdp-display-control.h   |  1 -
 src/grd-rdp-graphics-pipeline.c |  6 ------
 src/grd-rdp-graphics-pipeline.h |  1 -
 src/grd-rdp-telemetry.c         |  6 ------
 src/grd-rdp-telemetry.h         |  1 -
 src/grd-session-rdp.c           | 44 ++++++++++++++++++++++++-----------------
 9 files changed, 26 insertions(+), 46 deletions(-)
---
diff --git a/src/grd-rdp-audio-playback.c b/src/grd-rdp-audio-playback.c
index 0c222fe4..0075afee 100644
--- a/src/grd-rdp-audio-playback.c
+++ b/src/grd-rdp-audio-playback.c
@@ -60,7 +60,6 @@ struct _GrdRdpAudioPlayback
   GObject parent;
 
   RdpsndServerContext *rdpsnd_context;
-  HANDLE stop_event;
   gboolean channel_opened;
   gboolean channel_unavailable;
 
@@ -142,9 +141,6 @@ grd_rdp_audio_playback_maybe_init (GrdRdpAudioPlayback *audio_playback)
   if (audio_playback->channel_opened || audio_playback->channel_unavailable)
     return;
 
-  if (WaitForSingleObject (audio_playback->stop_event, 0) != WAIT_TIMEOUT)
-    return;
-
   rdpsnd_context = audio_playback->rdpsnd_context;
   if (rdpsnd_context->Start (rdpsnd_context))
     {
@@ -644,7 +640,6 @@ GrdRdpAudioPlayback *
 grd_rdp_audio_playback_new (GrdSessionRdp *session_rdp,
                             GrdRdpDvc     *rdp_dvc,
                             HANDLE         vcm,
-                            HANDLE         stop_event,
                             rdpContext    *rdp_context)
 {
   g_autoptr (GrdRdpAudioPlayback) audio_playback = NULL;
@@ -657,7 +652,6 @@ grd_rdp_audio_playback_new (GrdSessionRdp *session_rdp,
     g_error ("[RDP.AUDIO_PLAYBACK] Failed to create server context");
 
   audio_playback->rdpsnd_context = rdpsnd_context;
-  audio_playback->stop_event = stop_event;
   audio_playback->session_rdp = session_rdp;
   audio_playback->rdp_dvc = rdp_dvc;
 
diff --git a/src/grd-rdp-audio-playback.h b/src/grd-rdp-audio-playback.h
index 46fa2ff3..3d9dfd0e 100644
--- a/src/grd-rdp-audio-playback.h
+++ b/src/grd-rdp-audio-playback.h
@@ -32,7 +32,6 @@ G_DECLARE_FINAL_TYPE (GrdRdpAudioPlayback, grd_rdp_audio_playback,
 GrdRdpAudioPlayback *grd_rdp_audio_playback_new (GrdSessionRdp *session_rdp,
                                                  GrdRdpDvc     *rdp_dvc,
                                                  HANDLE         vcm,
-                                                 HANDLE         stop_event,
                                                  rdpContext    *rdp_context);
 
 void grd_rdp_audio_playback_maybe_init (GrdRdpAudioPlayback *audio_playback);
diff --git a/src/grd-rdp-display-control.c b/src/grd-rdp-display-control.c
index de271791..9df4241f 100644
--- a/src/grd-rdp-display-control.c
+++ b/src/grd-rdp-display-control.c
@@ -29,7 +29,6 @@ struct _GrdRdpDisplayControl
   GObject parent;
 
   DispServerContext *disp_context;
-  HANDLE stop_event;
   gboolean channel_opened;
   gboolean channel_unavailable;
 
@@ -53,9 +52,6 @@ grd_rdp_display_control_maybe_init (GrdRdpDisplayControl *display_control)
   if (display_control->channel_opened || display_control->channel_unavailable)
     return;
 
-  if (WaitForSingleObject (display_control->stop_event, 0) == WAIT_OBJECT_0)
-    return;
-
   disp_context = display_control->disp_context;
   if (disp_context->Open (disp_context))
     {
@@ -141,7 +137,6 @@ GrdRdpDisplayControl *
 grd_rdp_display_control_new (GrdSessionRdp *session_rdp,
                              GrdRdpDvc     *rdp_dvc,
                              HANDLE         vcm,
-                             HANDLE         stop_event,
                              uint32_t       max_monitor_count)
 {
   GrdRdpDisplayControl *display_control;
@@ -153,7 +148,6 @@ grd_rdp_display_control_new (GrdSessionRdp *session_rdp,
     g_error ("[RDP.DISP] Failed to create server context");
 
   display_control->disp_context = disp_context;
-  display_control->stop_event = stop_event;
   display_control->session_rdp = session_rdp;
   display_control->rdp_dvc = rdp_dvc;
 
diff --git a/src/grd-rdp-display-control.h b/src/grd-rdp-display-control.h
index 775fe30d..d79b20c5 100644
--- a/src/grd-rdp-display-control.h
+++ b/src/grd-rdp-display-control.h
@@ -32,7 +32,6 @@ G_DECLARE_FINAL_TYPE (GrdRdpDisplayControl, grd_rdp_display_control,
 GrdRdpDisplayControl *grd_rdp_display_control_new (GrdSessionRdp *session_rdp,
                                                    GrdRdpDvc     *rdp_dvc,
                                                    HANDLE         vcm,
-                                                   HANDLE         stop_event,
                                                    uint32_t       max_monitor_count);
 
 void grd_rdp_display_control_maybe_init (GrdRdpDisplayControl *display_control);
diff --git a/src/grd-rdp-graphics-pipeline.c b/src/grd-rdp-graphics-pipeline.c
index e16214e0..58bbc088 100644
--- a/src/grd-rdp-graphics-pipeline.c
+++ b/src/grd-rdp-graphics-pipeline.c
@@ -72,7 +72,6 @@ struct _GrdRdpGraphicsPipeline
   GObject parent;
 
   RdpgfxServerContext *rdpgfx_context;
-  HANDLE stop_event;
   gboolean channel_opened;
   gboolean received_first_cap_sets;
   gboolean initialized;
@@ -1464,9 +1463,6 @@ grd_rdp_graphics_pipeline_maybe_init (GrdRdpGraphicsPipeline *graphics_pipeline)
   if (graphics_pipeline->channel_opened)
     return;
 
-  if (WaitForSingleObject (graphics_pipeline->stop_event, 0) == WAIT_OBJECT_0)
-    return;
-
   rdpgfx_context = graphics_pipeline->rdpgfx_context;
   if (!rdpgfx_context->Open (rdpgfx_context))
     {
@@ -1492,7 +1488,6 @@ grd_rdp_graphics_pipeline_new (GrdSessionRdp              *session_rdp,
                                GrdRdpDvc                  *rdp_dvc,
                                GMainContext               *pipeline_context,
                                HANDLE                      vcm,
-                               HANDLE                      stop_event,
                                rdpContext                 *rdp_context,
                                GrdRdpNetworkAutodetection *network_autodetection,
                                wStream                    *encode_stream,
@@ -1507,7 +1502,6 @@ grd_rdp_graphics_pipeline_new (GrdSessionRdp              *session_rdp,
     g_error ("[RDP.RDPGFX] Failed to create server context");
 
   graphics_pipeline->rdpgfx_context = rdpgfx_context;
-  graphics_pipeline->stop_event = stop_event;
   graphics_pipeline->session_rdp = session_rdp;
   graphics_pipeline->rdp_dvc = rdp_dvc;
   graphics_pipeline->pipeline_context = pipeline_context;
diff --git a/src/grd-rdp-graphics-pipeline.h b/src/grd-rdp-graphics-pipeline.h
index 3adfc207..95151d6e 100644
--- a/src/grd-rdp-graphics-pipeline.h
+++ b/src/grd-rdp-graphics-pipeline.h
@@ -33,7 +33,6 @@ GrdRdpGraphicsPipeline *grd_rdp_graphics_pipeline_new (GrdSessionRdp
                                                        GrdRdpDvc                  *rdp_dvc,
                                                        GMainContext               *pipeline_context,
                                                        HANDLE                      vcm,
-                                                       HANDLE                      stop_event,
                                                        rdpContext                 *rdp_context,
                                                        GrdRdpNetworkAutodetection *network_autodetection,
                                                        wStream                    *encode_stream,
diff --git a/src/grd-rdp-telemetry.c b/src/grd-rdp-telemetry.c
index 1540fbe7..b8b15042 100644
--- a/src/grd-rdp-telemetry.c
+++ b/src/grd-rdp-telemetry.c
@@ -31,7 +31,6 @@ struct _GrdRdpTelemetry
   GObject parent;
 
   TelemetryServerContext *telemetry_context;
-  HANDLE stop_event;
   gboolean channel_opened;
   gboolean channel_unavailable;
 
@@ -74,9 +73,6 @@ grd_rdp_telemetry_maybe_init (GrdRdpTelemetry *telemetry)
   if (telemetry->channel_opened || telemetry->channel_unavailable)
     return;
 
-  if (WaitForSingleObject (telemetry->stop_event, 0) != WAIT_TIMEOUT)
-    return;
-
   telemetry_context = telemetry->telemetry_context;
   if (telemetry_context->Open (telemetry_context))
     {
@@ -173,7 +169,6 @@ GrdRdpTelemetry *
 grd_rdp_telemetry_new (GrdSessionRdp *session_rdp,
                        GrdRdpDvc     *rdp_dvc,
                        HANDLE         vcm,
-                       HANDLE         stop_event,
                        rdpContext    *rdp_context)
 {
   GrdRdpTelemetry *telemetry;
@@ -185,7 +180,6 @@ grd_rdp_telemetry_new (GrdSessionRdp *session_rdp,
     g_error ("[RDP.TELEMETRY] Failed to allocate server context (OOM)");
 
   telemetry->telemetry_context = telemetry_context;
-  telemetry->stop_event = stop_event;
   telemetry->session_rdp = session_rdp;
   telemetry->rdp_dvc = rdp_dvc;
 
diff --git a/src/grd-rdp-telemetry.h b/src/grd-rdp-telemetry.h
index 197577e1..49d756c4 100644
--- a/src/grd-rdp-telemetry.h
+++ b/src/grd-rdp-telemetry.h
@@ -32,7 +32,6 @@ G_DECLARE_FINAL_TYPE (GrdRdpTelemetry, grd_rdp_telemetry,
 GrdRdpTelemetry *grd_rdp_telemetry_new (GrdSessionRdp *session_rdp,
                                         GrdRdpDvc     *rdp_dvc,
                                         HANDLE         vcm,
-                                        HANDLE         stop_event,
                                         rdpContext    *rdp_context);
 
 void grd_rdp_telemetry_maybe_init (GrdRdpTelemetry *telemetry);
diff --git a/src/grd-session-rdp.c b/src/grd-session-rdp.c
index c2ff29ce..2c0bd4bb 100644
--- a/src/grd-session-rdp.c
+++ b/src/grd-session-rdp.c
@@ -139,7 +139,7 @@ struct _GrdSessionRdp
   GrdRdpSAMFile *sam_file;
   uint32_t rdp_error_info;
   GrdRdpScreenShareMode screen_share_mode;
-  gboolean session_stopped;
+  gboolean session_should_stop;
 
   SessionMetrics session_metrics;
 
@@ -654,6 +654,7 @@ maybe_queue_close_session_idle (GrdSessionRdp *session_rdp)
     g_idle_add (close_session_idle, session_rdp);
   g_mutex_unlock (&session_rdp->close_session_mutex);
 
+  session_rdp->session_should_stop = TRUE;
   SetEvent (session_rdp->stop_event);
 }
 
@@ -2144,7 +2145,7 @@ socket_thread_func (gpointer data)
 
       WaitForMultipleObjects (n_events, events, FALSE, INFINITE);
 
-      if (WaitForSingleObject (session_rdp->stop_event, 0) == WAIT_OBJECT_0)
+      if (session_rdp->session_should_stop)
         break;
 
       if (!peer->CheckFileDescriptor (peer))
@@ -2177,19 +2178,19 @@ socket_thread_func (gpointer data)
               audio_playback = rdp_peer_context->audio_playback;
               display_control = rdp_peer_context->display_control;
 
-              if (telemetry)
+              if (telemetry && !session_rdp->session_should_stop)
                 grd_rdp_telemetry_maybe_init (telemetry);
-              if (graphics_pipeline)
+              if (graphics_pipeline && !session_rdp->session_should_stop)
                 grd_rdp_graphics_pipeline_maybe_init (graphics_pipeline);
-              if (audio_playback)
+              if (audio_playback && !session_rdp->session_should_stop)
                 grd_rdp_audio_playback_maybe_init (audio_playback);
-              if (display_control)
+              if (display_control && !session_rdp->session_should_stop)
                 grd_rdp_display_control_maybe_init (display_control);
               g_mutex_unlock (&rdp_peer_context->channel_mutex);
               break;
             }
 
-          if (WaitForSingleObject (session_rdp->stop_event, 0) == WAIT_OBJECT_0)
+          if (session_rdp->session_should_stop)
             break;
         }
 
@@ -2225,7 +2226,7 @@ graphics_thread_func (gpointer data)
   if (session_rdp->hwaccel_nvidia)
     grd_hwaccel_nvidia_push_cuda_context (session_rdp->hwaccel_nvidia);
 
-  while (!session_rdp->session_stopped)
+  while (!session_rdp->session_should_stop)
     g_main_context_iteration (session_rdp->graphics_context, TRUE);
 
   if (session_rdp->hwaccel_nvidia)
@@ -2291,6 +2292,18 @@ grd_session_rdp_new (GrdRdpServer      *rdp_server,
   return g_steal_pointer (&session_rdp);
 }
 
+static gboolean
+has_session_close_queued (GrdSessionRdp *session_rdp)
+{
+  gboolean pending_session_closing;
+
+  g_mutex_lock (&session_rdp->close_session_mutex);
+  pending_session_closing = session_rdp->close_session_idle_id != 0;
+  g_mutex_unlock (&session_rdp->close_session_mutex);
+
+  return pending_session_closing;
+}
+
 static void
 clear_session_sources (GrdSessionRdp *session_rdp)
 {
@@ -2329,14 +2342,13 @@ grd_session_rdp_stop (GrdSession *session)
   g_debug ("Stopping RDP session");
 
   unset_rdp_peer_flag (session_rdp, RDP_PEER_ACTIVATED);
-  session_rdp->session_stopped = TRUE;
+  session_rdp->session_should_stop = TRUE;
+  SetEvent (session_rdp->stop_event);
 
-  if (WaitForSingleObject (session_rdp->stop_event, 0) == WAIT_TIMEOUT)
+  if (!has_session_close_queued (session_rdp))
     {
       freerdp_set_error_info (peer->context->rdp,
                               ERRINFO_RPC_INITIATED_DISCONNECT);
-
-      SetEvent (session_rdp->stop_event);
     }
   else if (session_rdp->rdp_error_info)
     {
@@ -2352,7 +2364,7 @@ grd_session_rdp_stop (GrdSession *session)
   if (session_rdp->graphics_thread)
     {
       g_assert (session_rdp->graphics_context);
-      g_assert (session_rdp->session_stopped);
+      g_assert (session_rdp->session_should_stop);
 
       g_main_context_wakeup (session_rdp->graphics_context);
       g_clear_pointer (&session_rdp->graphics_thread, g_thread_join);
@@ -2433,7 +2445,6 @@ maybe_initialize_graphics_pipeline (GrdSessionRdp *session_rdp)
   telemetry = grd_rdp_telemetry_new (session_rdp,
                                      rdp_peer_context->rdp_dvc,
                                      rdp_peer_context->vcm,
-                                     session_rdp->stop_event,
                                      peer->context);
   rdp_peer_context->telemetry = telemetry;
 
@@ -2442,7 +2453,6 @@ maybe_initialize_graphics_pipeline (GrdSessionRdp *session_rdp)
                                    rdp_peer_context->rdp_dvc,
                                    session_rdp->graphics_context,
                                    rdp_peer_context->vcm,
-                                   session_rdp->stop_event,
                                    peer->context,
                                    rdp_peer_context->network_autodetection,
                                    rdp_peer_context->encode_stream,
@@ -2473,8 +2483,7 @@ initialize_remaining_virtual_channels (GrdSessionRdp *session_rdp)
   if (rdp_settings->AudioPlayback && !rdp_settings->RemoteConsoleAudio)
     {
       rdp_peer_context->audio_playback =
-        grd_rdp_audio_playback_new (session_rdp, rdp_dvc, vcm,
-                                    session_rdp->stop_event, peer->context);
+        grd_rdp_audio_playback_new (session_rdp, rdp_dvc, vcm, peer->context);
     }
 }
 
@@ -2573,7 +2582,6 @@ grd_session_rdp_stream_ready (GrdSession *session,
             grd_rdp_display_control_new (session_rdp,
                                          rdp_peer_context->rdp_dvc,
                                          rdp_peer_context->vcm,
-                                         session_rdp->stop_event,
                                          MAX_MONITOR_COUNT);
         }
     }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]