Merge pull request #1992 from tyrielv/tyrielv/fix-rdp-telemetry-attach

Fix deferred telemetry pipe attach for RDP sessions

Author: tyrielv

GVFS/GVFS.Platform.Windows/CurrentUser.cs

@@ -134,7 +134,7 @@ public bool RunAs(string processName, string args)
                     if (CreateEnvironmentBlock(ref environment, duplicate, false))
                     {
                         STARTUP_INFO info = new STARTUP_INFO();
-                        info.Length = Marshal.SizeOf(typeof(STARTUP_INFO));
+                        info.Length = Marshal.SizeOf<STARTUP_INFO>();
 
                         PROCESS_INFORMATION procInfo = new PROCESS_INFORMATION();
                         if (CreateProcessAsUser(
@@ -193,6 +193,55 @@ public bool RunAs(string processName, string args)
             return false;
         }
 
+        /// <summary>
+        /// Returns session IDs for sessions that have a logged-in user
+        /// whose token can be queried via <c>WTSQueryUserToken</c>.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// WTS session states relevant to telemetry pipe attachment:
+        /// </para>
+        /// <list type="table">
+        /// <listheader><term>State</term><description>Meaning</description></listheader>
+        /// <item><term>Active</term><description>
+        ///   User logged in, session connected (console or RDP).
+        ///   Has a valid user token. This is the primary case.
+        /// </description></item>
+        /// <item><term>Connected</term><description>
+        ///   Client attached but no user logged in (e.g. the console
+        ///   session showing the Windows login screen on a Cloud PC).
+        ///   No user token — <c>WTSQueryUserToken</c> will fail.
+        /// </description></item>
+        /// <item><term>Disconnected</term><description>
+        ///   User logged in but client disconnected (e.g. closed RDP
+        ///   window without logging off). The user's profile, processes,
+        ///   and token are still available. Included so the service can
+        ///   attach telemetry even when no client is actively connected.
+        /// </description></item>
+        /// </list>
+        /// <para>
+        /// Session 0 is the services session and never has a user token,
+        /// even when its state is Disconnected. Excluded by the ID > 0
+        /// guard.
+        /// </para>
+        /// </remarks>
+        public static List<int> GetInteractiveSessionIds(ITracer tracer)
+        {
+            List<int> sessionIds = new List<int>();
+            List<WTS_SESSION_INFO> sessions = ListSessions(tracer);
+            foreach (WTS_SESSION_INFO session in sessions)
+            {
+                if (session.SessionID > 0 &&
+                    (session.State == ConnectionState.Active ||
+                     session.State == ConnectionState.Disconnected))
+                {
+                    sessionIds.Add(session.SessionID);
+                }
+            }
+
+            return sessionIds;
+        }
+
         public void Dispose()
         {
             if (this.token != IntPtr.Zero)
@@ -216,7 +265,10 @@ private static IntPtr GetCurrentUserToken(ITracer tracer, int sessionId)
             }
             else
             {
-                TraceWin32Error(tracer, string.Format("Unable to query user token from session '{0}'.", sessionId));
+                // Warning, not error: sessions without a logged-in user
+                // (e.g. the console session on a Cloud PC) are expected.
+                Win32Exception e = new Win32Exception(Marshal.GetLastWin32Error());
+                tracer.RelatedWarning("Unable to query user token from session '{0}'. Exception: {1}", sessionId, e.Message);
             }
 
             return IntPtr.Zero;
@@ -234,12 +286,12 @@ private static List<WTS_SESSION_INFO> ListSessions(ITracer tracer)
                 int retval = WTSEnumerateSessions(IntPtr.Zero, 0, 1, ref sessionInfo, ref count);
                 if (retval != 0)
                 {
-                    int dataSize = Marshal.SizeOf(typeof(WTS_SESSION_INFO));
+                    int dataSize = Marshal.SizeOf<WTS_SESSION_INFO>();
                     long current = sessionInfo.ToInt64();
 
                     for (int i = 0; i < count; i++)
                     {
-                        WTS_SESSION_INFO si = (WTS_SESSION_INFO)Marshal.PtrToStructure((IntPtr)current, typeof(WTS_SESSION_INFO));
+                        WTS_SESSION_INFO si = Marshal.PtrToStructure<WTS_SESSION_INFO>((IntPtr)current);
                         current += dataSize;
 
                         output.Add(si);

GVFS/GVFS.Service/GVFSService.Windows.cs

@@ -5,6 +5,7 @@
 using GVFS.Platform.Windows;
 using GVFS.Service.Handlers;
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Security.AccessControl;
@@ -46,6 +47,46 @@ public void Run()
                 metadata.Add("Version", ProcessHelper.GetCurrentProcessVersion());
                 this.tracer.RelatedEvent(EventLevel.Informational, $"{nameof(GVFSService)}_{nameof(this.Run)}", metadata);
 
+                // Set up deferred telemetry pipe attachment FIRST, before any
+                // telemetry-emitting work (particularly PendingUpgradeHandler,
+                // whose Deferred/Complete events we want to capture).
+                //
+                // The service runs as SYSTEM and can't read the user's global
+                // git config (where gvfs.telemetry-pipe is configured) at
+                // startup.  The DeferredTelemetryAttacher adds a
+                // BufferingTelemetryListener that captures events in memory,
+                // then replays them once the real pipe listener attaches.
+                //
+                // Three attach paths exist:
+                //   1. TryAttachTelemetryPipeForAnySessions() below — tries
+                //      all Active/Disconnected sessions immediately.
+                //   2. OnSessionChange (SessionLogon) — fires when a new user
+                //      logs in after the service is already running.
+                //   3. StartRetryTimer — periodic retry (10s, 30s, 1m, 5m)
+                //      as a fallback, reads system config only (no user
+                //      config available without a session to impersonate).
+                string gitBinRoot = GVFSPlatform.Instance.GitInstallation.GetInstalledGitBinPath();
+                if (!string.IsNullOrEmpty(gitBinRoot))
+                {
+                    this.telemetryAttacher = new DeferredTelemetryAttacher(
+                        this.tracer,
+                        GVFSConstants.Service.ServiceName,
+                        enlistmentId: null,
+                        mountId: null);
+                    this.telemetryAttacher.StartRetryTimer(gitBinRoot);
+
+                    // If a user is already logged in (e.g. service restart
+                    // during active session), try attaching immediately by
+                    // enumerating all interactive sessions.  This is needed
+                    // because WTSGetActiveConsoleSessionId only returns the
+                    // physical console session, which on Cloud PCs / DevBoxes
+                    // (RDP-only) has no logged-in user.  The actual user is
+                    // in an RDP session that the console-only check misses.
+                    // SessionLogon events also won't fire for sessions that
+                    // were already established before the service started.
+                    this.TryAttachTelemetryPipeForAnySessions();
+                }
+
                 // Check for a staged upgrade before doing anything else.
                 // If no GVFS.Mount processes are running (typical at boot or after
                 // unmount-all), copy staged files in-place and proceed normally.
@@ -77,29 +118,6 @@ public void Run()
                 {
                     this.CheckEnableGitStatusCacheTokenFile();
 
-                    // Set up deferred telemetry pipe attachment.  The service
-                    // runs as SYSTEM and can't read the user's global git
-                    // config at startup, so the daemon listener can't be
-                    // created in the JsonTracer constructor.
-                    string gitBinRoot = GVFSPlatform.Instance.GitInstallation.GetInstalledGitBinPath();
-                    if (!string.IsNullOrEmpty(gitBinRoot))
-                    {
-                        this.telemetryAttacher = new DeferredTelemetryAttacher(
-                            this.tracer,
-                            GVFSConstants.Service.ServiceName,
-                            enlistmentId: null,
-                            mountId: null);
-                        this.telemetryAttacher.StartRetryTimer(gitBinRoot);
-
-                        // If a user is already logged in (e.g. service restart
-                        // during active session), try attaching immediately.
-                        int activeSession = NativeMethods.GetActiveConsoleSessionId();
-                        if (activeSession > 0)
-                        {
-                            this.TryAttachTelemetryPipeForSession(activeSession);
-                        }
-                    }
-
                     using (ITracer activity = this.tracer.StartActivity("EnsurePrjFltHealthy", EventLevel.Informational))
                     {
                         // Make a best-effort to enable PrjFlt. Continue even if it fails.
@@ -475,6 +493,50 @@ private void TryAttachTelemetryPipeForSession(int sessionId)
             }
         }
 
+        /// <summary>
+        /// Enumerates all interactive sessions (Active and Disconnected)
+        /// and tries to attach the telemetry pipe using each session's
+        /// user profile.  Stops on the first successful attach.
+        /// </summary>
+        /// <remarks>
+        /// This method exists because the console-only check
+        /// (<c>WTSGetActiveConsoleSessionId</c>) fails on Cloud PCs and
+        /// RDP-only machines where the console session is in the Connected
+        /// state (login screen, no user).  Disconnected sessions are also
+        /// checked because an RDP user who disconnected without logging
+        /// off still has a valid token and git config.
+        /// </remarks>
+        private void TryAttachTelemetryPipeForAnySessions()
+        {
+            if (this.telemetryAttacher == null || this.telemetryAttacher.IsAttached)
+            {
+                return;
+            }
+
+            List<int> sessionIds = CurrentUser.GetInteractiveSessionIds(this.tracer);
+            if (sessionIds.Count == 0)
+            {
+                this.tracer.RelatedInfo("TryAttachTelemetryPipeForAnySessions: No interactive sessions found");
+                return;
+            }
+
+            foreach (int sessionId in sessionIds)
+            {
+                this.TryAttachTelemetryPipeForSession(sessionId);
+                if (this.telemetryAttacher.IsAttached)
+                {
+                    break;
+                }
+            }
+
+            if (!this.telemetryAttacher.IsAttached)
+            {
+                this.tracer.RelatedWarning(
+                    "TryAttachTelemetryPipeForAnySessions: Could not attach from any of {0} interactive session(s)",
+                    sessionIds.Count);
+            }
+        }
+
         private DirectorySecurity GetServiceDirectorySecurity(string serviceDataRootPath)
         {
             DirectorySecurity serviceDataRootSecurity;