[hbelusca] 65799: In absence of patchbot, let's test patch directly on... trunk!! Yeah!! Let's break trunk! (or not, we'll see...) [USER32][USERSRV][WIN32K] - Flag CSRSS threads as such. - Each tim...

Author: hbelusca
Date: Mon Dec 22 21:23:00 2014
New Revision: 65799

URL: http://svn.reactos.org/svn/reactos?rev=65799&view=rev
Log:
In absence of patchbot, let's test patch directly on... trunk!! Yeah!! Let's break trunk! (or not, we'll see...)

[USER32][USERSRV][WIN32K]
- Flag CSRSS threads as such.
- Each time a win32 thread is "created" (or a win32k system call is done) a PsConvertToGuiThread call is done, that makes a Win32ThreadCallout which calls UserCreateThreadInfo. We should call back ClientThreadSetup to set client-side info.
- Introduce a ClientThreadSetupHelper function to try to understand what happens when doing direct calls to ClientThreadSetup and via win32k callbacks.
- Reenable CSR call in user32::DllMain but add shame hacks in Init(..) to force NtUserProcessConnect callbacks (yet they should not be done there): see the code comment associated for more details.
- Temporarily disable ClientThreadSetupHelper call in ClientThreadSetup because it makes things fail, like console input thread failing and so on...
- ... and in ClientThreadSetupHelper, some correct code is commented out because it currently breaks other things (like menu initialization etc...), and HACKS ARE ADDED!! (see especially the "if (gpsi)" one, that should not exist, but if removed we fail sometimes in MenuInit because gpsi becomes NULL for some reason, if some other code that makes redundant calls to NtUserProcessConnect, is uncommented).

To sum up I tried to scaffold the thing as it should be, but with disabled code to not break everything.

CORE-8949 #comment Revision 65799 committed the patch v1.

Modified:
    trunk/reactos/win32ss/user/ntuser/main.c
    trunk/reactos/win32ss/user/ntuser/ntuser.c
    trunk/reactos/win32ss/user/user32/misc/dllmain.c
    trunk/reactos/win32ss/user/winsrv/usersrv/init.c

Modified: trunk/reactos/win32ss/user/ntuser/main.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/main.c?...
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/main.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/main.c	[iso-8859-1] Mon Dec 22 21:23:00 2014
@@ -385,12 +385,19 @@
         Status = STATUS_NO_MEMORY;
         goto error;
     }
+
     ptiCurrent->KeyboardLayout = W32kGetDefaultKeyLayout();
     if (ptiCurrent->KeyboardLayout)
         UserReferenceObject(ptiCurrent->KeyboardLayout);
+
     ptiCurrent->TIF_flags &= ~TIF_INCLEANUP;
-    if (Process == gpepCSRSS) /* If this thread is owned by CSRSS, mark it as such */
-        ptiCurrent->TIF_flags |= TIF_CSRSSTHREAD;
+
+    /* CSRSS threads have some special features */
+    if (Process == gpepCSRSS)
+        ptiCurrent->TIF_flags = TIF_CSRSSTHREAD | TIF_DONTATTACHQUEUE;
+
+    // FIXME: Flag SYSTEM threads with... TIF_SYSTEMTHREAD !!
+
     ptiCurrent->pcti = &ptiCurrent->cti;

/* Initialize the CLIENTINFO */
@@ -475,7 +482,7 @@
         }
     }

-    /* mark the thread as fully initialized */
+    /* Mark the thread as fully initialized */
     ptiCurrent->TIF_flags |= TIF_GUITHREADINITIALIZED;

if (!(ptiCurrent->ppi->W32PF_flags & (W32PF_ALLOWFOREGROUNDACTIVATE | W32PF_APPSTARTING)) &&
@@ -484,7 +491,26 @@
        ptiCurrent->TIF_flags |= TIF_ALLOWFOREGROUNDACTIVATE;
     }
     ptiCurrent->pClientInfo->dwTIFlags = ptiCurrent->TIF_flags;
-    TRACE_CH(UserThread,"UserCreateW32Thread pti 0x%p\n",ptiCurrent);
+
+    /* Last things to do only if we are not a SYSTEM or CSRSS thread */
+    if (!(ptiCurrent->TIF_flags & (TIF_SYSTEMTHREAD | TIF_CSRSSTHREAD)))
+    {
+        /* Callback to User32 Client Thread Setup */
+        TRACE_CH(UserThread,"Call co_IntClientThreadSetup...\n");
+        Status = co_IntClientThreadSetup();
+        if (!NT_SUCCESS(Status))
+        {
+            ERR_CH(UserThread,"ClientThreadSetup failed with Status 0x%08lx\n", Status);
+            goto error;
+        }
+        TRACE_CH(UserThread,"co_IntClientThreadSetup succeeded!\n");
+    }
+    else
+    {
+        TRACE_CH(UserThread,"co_IntClientThreadSetup cannot be called...\n");
+    }
+
+    TRACE_CH(UserThread,"UserCreateW32Thread pti 0x%p\n", ptiCurrent);
     return STATUS_SUCCESS;

error:

Modified: trunk/reactos/win32ss/user/ntuser/ntuser.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/ntuser....
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/ntuser.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/ntuser.c	[iso-8859-1] Mon Dec 22 21:23:00 2014
@@ -128,10 +128,6 @@
     Status = UserCreateThreadInfo(PsGetCurrentThread());
     if (!NT_SUCCESS(Status)) return Status;

-//    Callback to User32 Client Thread Setup
-
-    co_IntClientThreadSetup();
-
 // }
 // Set Global SERVERINFO Error flags.
 // Load Resources.

Modified: trunk/reactos/win32ss/user/user32/misc/dllmain.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/user32/misc/dl...
==============================================================================
--- trunk/reactos/win32ss/user/user32/misc/dllmain.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/user32/misc/dllmain.c	[iso-8859-1] Mon Dec 22 21:23:00 2014
@@ -17,6 +17,7 @@
 ULONG_PTR g_ulSharedDelta;
 BOOLEAN gfLogonProcess  = FALSE;
 BOOLEAN gfServerProcess = FALSE;
+BOOLEAN gfFirstThread   = TRUE;
 HICON hIconSmWindows = NULL, hIconWindows = NULL;

WCHAR szAppInit[KEY_LENGTH];
@@ -203,9 +204,119 @@
     User32DeliverUserAPC,
 };

+
+
+VOID
+WINAPI
+GdiProcessSetup(VOID);
+
+BOOL
+WINAPI
+ClientThreadSetupHelper(BOOL IsCallback)
+{
+    /*
+     * Normally we are called by win32k so the win32 thread pointers
+     * should be valid as they are set in win32k::UserCreateThreadInfo.
+     */
+    PCLIENTINFO ClientInfo = GetWin32ClientInfo();
+    BOOLEAN IsFirstThread = _InterlockedExchange8((PCHAR)&gfFirstThread, FALSE);
+
+    TRACE("In ClientThreadSetup(IsCallback == %s, gfServerProcess = %s, IsFirstThread = %s)\n",
+          IsCallback ? "TRUE" : "FALSE", gfServerProcess ? "TRUE" : "FALSE", IsFirstThread ? "TRUE" : "FALSE");
+
+    if (IsFirstThread)
+        GdiProcessSetup();
+
+    /* Check for already initialized thread, and bail out if so */
+    if (ClientInfo->CI_flags & CI_INITTHREAD)
+    {
+        ERR("ClientThreadSetup: Thread already initialized.\n");
+        return FALSE;
+    }
+
+    /*
+     * CSRSS couldn't use user32::DllMain CSR server-to-server call to connect
+     * to win32k. So it is delayed to a manually-call to ClientThreadSetup.
+     * Also this needs to be done only for the first thread (since the connection
+     * is per-process).
+     */
+    // if (gfServerProcess && IsFirstThread) // Disabling this test is a HACK!!
+    {
+        NTSTATUS Status;
+        USERCONNECT UserCon;
+
+        RtlZeroMemory(&UserCon, sizeof(UserCon));
+
+        /* Minimal setup of the connect info structure */
+        UserCon.ulVersion = USER_VERSION;
+
+        /* Connect to win32k */
+        Status = NtUserProcessConnect(NtCurrentProcess(),
+                                      &UserCon,
+                                      sizeof(UserCon));
+        if (!NT_SUCCESS(Status)) return FALSE;
+
+        /* Retrieve data */
+        g_ppi = ClientInfo->ppi; // Snapshot PI, used as pointer only!
+        g_ulSharedDelta = UserCon.siClient.ulSharedDelta;
+        gpsi = SharedPtrToUser(UserCon.siClient.psi);
+        gHandleTable = SharedPtrToUser(UserCon.siClient.aheList);
+        gHandleEntries = SharedPtrToUser(gHandleTable->handles);
+
+        // ERR("1 SI 0x%x : HT 0x%x : D 0x%x\n", UserCon.siClient.psi, UserCon.siClient.aheList,  g_ulSharedDelta);
+    }
+
+    TRACE("Checkpoint (register PFN)\n");
+    if (!RegisterClientPFN())
+    {
+        TRACE("RegisterClientPFN failed\n");
+        return FALSE;
+    }
+
+    /* Mark this thread as initialized */
+    ClientInfo->CI_flags |= CI_INITTHREAD;
+
+    /* Initialization that should be done once per process */
+    if (IsFirstThread)
+    {
+        TRACE("Checkpoint (Allocating TLS)\n");
+
+        /* Allocate an index for user32 thread local data */
+        User32TlsIndex = TlsAlloc();
+        if (User32TlsIndex == TLS_OUT_OF_INDEXES)
+            return FALSE;
+
+        // HAAAAAAAAAACK!!!!!!
+        // ASSERT(gpsi);
+        if (!gpsi) ERR("AAAAAAAAAAAHHHHHHHHHHHHHH!!!!!!!! gpsi == NULL !!!!\n");
+        if (gpsi)
+        {
+        TRACE("Checkpoint (MessageInit)\n");
+
+        if (MessageInit())
+        {
+            TRACE("Checkpoint (MenuInit)\n");
+            if (MenuInit())
+            {
+                TRACE("Checkpoint initialization done OK\n");
+                InitializeCriticalSection(&U32AccelCacheLock);
+                LoadAppInitDlls();
+                return TRUE;
+            }
+            MessageCleanup();
+        }
+
+        TlsFree(User32TlsIndex);
+        return FALSE;
+        }
+    }
+
+    return TRUE;
+}
+
 /*
-* @unimplemented
-*/
+ * @implemented
+ */
 BOOL
 WINAPI
 ClientThreadSetup(VOID)
@@ -221,7 +332,7 @@
     // However, all the other calls are made as normal, and WINSRV.DLL loads
     // USER32.dll, the DllMain runs, and eventually the first NtUser system call is
     // made which initializes Win32k (and initializes the thread, but as mentioned
-    // above, the thread is marked as TIF_CSRSSTHREAD.
+    // above, the thread is marked as TIF_CSRSSTHREAD).
     //
     // In the DllMain of User32, there is also a CsrClientConnectToServer call to
     // server 2 (winsrv). When this is done from CSRSS, the "InServer" flag is set,
@@ -241,73 +352,74 @@
     // continue as normal.
     //

-    // GetConnected();
-
-    UNIMPLEMENTED;
+    // return ClientThreadSetupHelper(FALSE); // Disabling this call is a HACK!!
     return TRUE;
 }

BOOL
-Init(VOID)
-{
-    NTSTATUS Status;
-    USERCONNECT UserCon;
-
-    TRACE("user32::Init()\n");
-
-    /* Set PEB data */
+Init(PUSERCONNECT UserCon /*PUSERSRV_API_CONNECTINFO*/)
+{
+    NTSTATUS Status = STATUS_SUCCESS;
+
+    TRACE("user32::Init(0x%p) -->\n", UserCon);
+
+    RtlInitializeCriticalSection(&gcsUserApiHook);
+
+    /* Initialize callback table in PEB data */
     NtCurrentPeb()->KernelCallbackTable = apfnDispatch;
     NtCurrentPeb()->PostProcessInitRoutine = NULL;

-    /* Minimal setup of the connect info structure */
-    UserCon.ulVersion = USER_VERSION;
-
-    /* Connect to win32k */
-    Status = NtUserProcessConnect(NtCurrentProcess(),
-                                  &UserCon,
-                                  sizeof(UserCon));
-    if (!NT_SUCCESS(Status)) return FALSE;
-
-    /* Retrieve data */
-    g_ppi = GetWin32ClientInfo()->ppi; // Snapshot PI, used as pointer only!
-    g_ulSharedDelta = UserCon.siClient.ulSharedDelta;
-    gpsi = SharedPtrToUser(UserCon.siClient.psi);
-    gHandleTable = SharedPtrToUser(UserCon.siClient.aheList);
-    gHandleEntries = SharedPtrToUser(gHandleTable->handles);
-
-    RtlInitializeCriticalSection(&gcsUserApiHook);
-
-    //ERR("1 SI 0x%x : HT 0x%x : D 0x%x\n", UserCon.siClient.psi, UserCon.siClient.aheList,  g_ulSharedDelta);
-
-    /* Allocate an index for user32 thread local data */
-    User32TlsIndex = TlsAlloc();
-    if (User32TlsIndex == TLS_OUT_OF_INDEXES)
-        return FALSE;
-
-    if (MessageInit())
-    {
-        if (MenuInit())
+    /*
+     * Retrieve data from the connect info structure if the initializing
+     * process is not CSRSS. In case it is, this will be done from inside
+     * ClientThreadSetup.
+     */
+    // if (!gfServerProcess) // Disabling this test is a HACK!!
+    {
+        // FIXME: HACK!! We should fixup for the NtUserProcessConnect fixups
+        // because it was made in the context of CSRSS process and not ours!!
+        // So... as long as we don't fix that, we need to redo again a call
+        // to NtUserProcessConnect... How perverse is that?!
         {
-            InitializeCriticalSection(&U32AccelCacheLock);
-            LoadAppInitDlls();
-            return TRUE;
+            RtlZeroMemory(UserCon, sizeof(*UserCon));
+
+            /* Minimal setup of the connect info structure */
+            UserCon->ulVersion = USER_VERSION;
+
+            ERR("HACK: Hackish NtUserProcessConnect call!!\n");
+            /* Connect to win32k */
+            Status = NtUserProcessConnect(NtCurrentProcess(),
+                                          UserCon,
+                                          sizeof(*UserCon));
+            if (!NT_SUCCESS(Status)) return FALSE;
         }
-        MessageCleanup();
-    }
-
-    TlsFree(User32TlsIndex);
-    return FALSE;
+
+        //
+        // We continue as we should do...
+        //
+
+        /* Retrieve data */
+        g_ppi = GetWin32ClientInfo()->ppi; // Snapshot PI, used as pointer only!
+        g_ulSharedDelta = UserCon->siClient.ulSharedDelta;
+        gpsi = SharedPtrToUser(UserCon->siClient.psi);
+        gHandleTable = SharedPtrToUser(UserCon->siClient.aheList);
+        gHandleEntries = SharedPtrToUser(gHandleTable->handles);
+    }
+
+    TRACE("<-- user32::Init()\n");
+
+    return NT_SUCCESS(Status);
 }

VOID
 Cleanup(VOID)
 {
+    UnloadAppInitDlls();
     DeleteCriticalSection(&U32AccelCacheLock);
     MenuCleanup();
     MessageCleanup();
+    TlsFree(User32TlsIndex);
     DeleteFrameBrushes();
-    UnloadAppInitDlls();
-    TlsFree(User32TlsIndex);
 }

INT WINAPI
@@ -321,8 +433,6 @@
         case DLL_PROCESS_ATTACH:
         {

-#if 0
-
 #define WIN_OBJ_DIR L"\Windows"
 #define SESSION_DIR L"\Sessions"

@@ -335,8 +445,12 @@
             PPEB Peb = NtCurrentPeb();
             ULONG SessionId = Peb->SessionId; // gSessionId

+            TRACE("user32::DllMain\n");
+
             /* Don't bother us for each thread */
             DisableThreadLibraryCalls(hInstanceDll);
+
+            RtlZeroMemory(&ConnectInfo, sizeof(ConnectInfo));

/* Minimal setup of the connect info structure */
             ConnectInfo.ulVersion = USER_VERSION;
@@ -357,6 +471,8 @@
                          WIN_OBJ_DIR);
             }

+            TRACE("Checkpoint (call CSR)\n");
+
             /* Connect to the USER Server */
             Status = CsrClientConnectToServer(SessionDir,
                                               USERSRV_SERVERDLL_INDEX,
@@ -369,16 +485,13 @@
                 return FALSE;
             }

-#else
-    gfServerProcess = FALSE;
-#endif
+            TRACE("Checkpoint (CSR called)\n");

User32Instance = hInstanceDll;

-            if (!RegisterClientPFN())
-                return FALSE;
-
-            if (!Init())
+            /* Finish initialization */
+            TRACE("Checkpoint (call Init)\n");
+            if (!Init(&ConnectInfo))
                 return FALSE;

break;
@@ -398,44 +511,13 @@
     return GdiDllInitialize(hInstanceDll, dwReason, reserved);
 }

-// FIXME: This function IS A BIIG HACK!!
-VOID
-FASTCALL
-GetConnected(VOID)
-{
-    NTSTATUS Status;
-    USERCONNECT UserCon;
-
-    TRACE("user32::GetConnected()\n");
-
-    if ((PTHREADINFO)NtCurrentTeb()->Win32ThreadInfo == NULL)
-        NtUserGetThreadState(THREADSTATE_GETTHREADINFO);
-
-    if (gpsi && g_ppi) return;
-
-    /* Minimal setup of the connect info structure */
-    UserCon.ulVersion = USER_VERSION;
-
-    /* Connect to win32k */
-    Status = NtUserProcessConnect(NtCurrentProcess(),
-                                  &UserCon,
-                                  sizeof(UserCon));
-    if (!NT_SUCCESS(Status)) return;
-
-    /* Retrieve data */
-    g_ppi = GetWin32ClientInfo()->ppi; // Snapshot PI, used as pointer only!
-    g_ulSharedDelta = UserCon.siClient.ulSharedDelta;
-    gpsi = SharedPtrToUser(UserCon.siClient.psi);
-    gHandleTable = SharedPtrToUser(UserCon.siClient.aheList);
-    gHandleEntries = SharedPtrToUser(gHandleTable->handles);
-}
-
 NTSTATUS
 WINAPI
 User32CallClientThreadSetupFromKernel(PVOID Arguments, ULONG ArgumentLength)
 {
-  TRACE("ClientThreadSetup\n");
-  ClientThreadSetup();
+  TRACE("User32CallClientThreadSetupFromKernel -->\n");
+  ClientThreadSetupHelper(TRUE);
+  TRACE("<-- User32CallClientThreadSetupFromKernel\n");
   return ZwCallbackReturn(NULL, 0, STATUS_SUCCESS);
 }

Modified: trunk/reactos/win32ss/user/winsrv/usersrv/init.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/winsrv/usersrv...
==============================================================================
--- trunk/reactos/win32ss/user/winsrv/usersrv/init.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/winsrv/usersrv/init.c	[iso-8859-1] Mon Dec 22 21:23:00 2014
@@ -147,7 +147,7 @@
     // PUSERCONNECT
     PUSERSRV_API_CONNECTINFO ConnectInfo = (PUSERSRV_API_CONNECTINFO)ConnectionInfo;

-    DPRINT1("UserClientConnect\n");
+    DPRINT("UserClientConnect\n");

/* Check if we don't have an API port yet */
     if (CsrApiPort == NULL)