[ros-diffs] [hbelusca] 58166: [KERNEL32] - Correct a error return code (in AllocConsole). - Implement AttachConsole (inspired by AllocConsole). [CONSRV] - Correct a error return code (in AllocConsole). - Intro...

Author: hbelusca
Date: Sun Jan 13 17:07:25 2013
New Revision: 58166

URL: http://svn.reactos.org/svn/reactos?rev=58166&view=rev
Log:
[KERNEL32]
- Correct a error return code (in AllocConsole).
- Implement AttachConsole (inspired by AllocConsole).

[CONSRV]
- Correct a error return code (in AllocConsole).
- Introduce helper functions to simplify console-initialization code.
- Implement server-side of AttachConsole (I used pieces of code from ConsoleNewProcess, ConsoleConnect and SrvAllocConsole).

Some debug prints were introduced (for debug-purposes :P), will be removed when all things work.

TODO: have a look on the handles code.

Modified:
    branches/ros-csrss/dll/win32/kernel32/client/console/console.c
    branches/ros-csrss/include/reactos/subsys/win/conmsg.h
    branches/ros-csrss/win32ss/user/consrv/console.c
    branches/ros-csrss/win32ss/user/consrv/consrv.h
    branches/ros-csrss/win32ss/user/consrv/handle.c
    branches/ros-csrss/win32ss/user/consrv/init.c

Modified: branches/ros-csrss/dll/win32/kernel32/client/console/console.c
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/dll/win32/kernel32/cli...
==============================================================================
--- branches/ros-csrss/dll/win32/kernel32/client/console/console.c [iso-8859-1] (original)
+++ branches/ros-csrss/dll/win32/kernel32/client/console/console.c [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -802,8 +802,8 @@

if (NtCurrentPeb()->ProcessParameters->ConsoleHandle)
     {
-        DPRINT("AllocConsole: Allocate duplicate console to the same Process\n");
-        BaseSetLastNTError (STATUS_OBJECT_NAME_EXISTS);
+        DPRINT1("AllocConsole: Allocate duplicate console to the same Process\n");
+        SetLastError(ERROR_ACCESS_DENIED);
         return FALSE;
     }

@@ -824,7 +824,6 @@
     }

NtCurrentPeb()->ProcessParameters->ConsoleHandle = AllocConsoleRequest->Console;
-
     SetStdHandle(STD_INPUT_HANDLE , AllocConsoleRequest->InputHandle );
     SetStdHandle(STD_OUTPUT_HANDLE, AllocConsoleRequest->OutputHandle);
     SetStdHandle(STD_ERROR_HANDLE , AllocConsoleRequest->ErrorHandle );
@@ -1910,13 +1909,50 @@
 /*--------------------------------------------------------------
  *     AttachConsole
  *
- * @unimplemented
+ * @implemented
+ *
+ * @note Strongly inspired by AllocConsole.
  */
 BOOL
 WINAPI
 AttachConsole(DWORD dwProcessId)
 {
-    DPRINT1("AttachConsole(0x%x) UNIMPLEMENTED!\n", dwProcessId);
+    NTSTATUS Status;
+    CONSOLE_API_MESSAGE ApiMessage;
+    PCONSOLE_ATTACHCONSOLE AttachConsoleRequest = &ApiMessage.Data.AttachConsoleRequest;
+
+    DPRINT1("AttachConsole(%lu)\n", dwProcessId);
+
+    if (NtCurrentPeb()->ProcessParameters->ConsoleHandle)
+    {
+        DPRINT1("AttachConsole: Attaching a console to a process already having one\n");
+        SetLastError(ERROR_ACCESS_DENIED);
+        return FALSE;
+    }
+
+    AttachConsoleRequest->ProcessId = dwProcessId;
+    AttachConsoleRequest->CtrlDispatcher = ConsoleControlDispatcher;
+
+    Status = CsrClientCallServer((PCSR_API_MESSAGE)&ApiMessage,
+                                 NULL,
+                                 CSR_CREATE_API_NUMBER(CONSRV_SERVERDLL_INDEX, ConsolepAttach),
+                                 sizeof(CONSOLE_ATTACHCONSOLE));
+    if (!NT_SUCCESS(Status) || !NT_SUCCESS(Status = ApiMessage.Status))
+    {
+        BaseSetLastNTError(Status);
+        return FALSE;
+    }
+
+    NtCurrentPeb()->ProcessParameters->ConsoleHandle = AttachConsoleRequest->Console;
+    SetStdHandle(STD_INPUT_HANDLE , AttachConsoleRequest->InputHandle );
+    SetStdHandle(STD_OUTPUT_HANDLE, AttachConsoleRequest->OutputHandle);
+    SetStdHandle(STD_ERROR_HANDLE , AttachConsoleRequest->ErrorHandle );
+
+    /* Initialize Console Ctrl Handler */
+    InitConsoleCtrlHandling();
+
+    InputWaitHandle = AttachConsoleRequest->InputWaitHandle;
+
     return TRUE;
 }

Modified: branches/ros-csrss/include/reactos/subsys/win/conmsg.h
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/include/reactos/subsys...
==============================================================================
--- branches/ros-csrss/include/reactos/subsys/win/conmsg.h [iso-8859-1] (original)
+++ branches/ros-csrss/include/reactos/subsys/win/conmsg.h [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -102,7 +102,7 @@
     // ConsolepRegisterConsoleIME,
     // ConsolepUnregisterConsoleIME,
     // ConsolepGetLangId,
-    // ConsolepAttach,
+    ConsolepAttach,
     ConsolepGetSelectionInfo,
     ConsolepGetProcessList,
     ConsolepGetHistory,
@@ -176,6 +176,17 @@
     HANDLE InputWaitHandle;
     LPTHREAD_START_ROUTINE CtrlDispatcher;
 } CONSOLE_ALLOCCONSOLE, *PCONSOLE_ALLOCCONSOLE;
+
+typedef struct
+{
+    DWORD ProcessId; // If ProcessId == ATTACH_PARENT_PROCESS == -1, then attach the current process to its parent process console.
+    HANDLE Console; // ConsoleHandle // In fact, it is a PCSRSS_CONSOLE <-- correct that !!
+    HANDLE InputHandle;
+    HANDLE OutputHandle;
+    HANDLE ErrorHandle;
+    HANDLE InputWaitHandle;
+    LPTHREAD_START_ROUTINE CtrlDispatcher;
+} CONSOLE_ATTACHCONSOLE, *PCONSOLE_ATTACHCONSOLE;

typedef struct
 {
@@ -543,6 +554,7 @@
     union
     {
         CONSOLE_ALLOCCONSOLE AllocConsoleRequest;
+        CONSOLE_ATTACHCONSOLE AttachConsoleRequest;
         CONSOLE_FREECONSOLE FreeConsoleRequest;

/* Handles */

Modified: branches/ros-csrss/win32ss/user/consrv/console.c
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/win32ss/user/consrv/co...
==============================================================================
--- branches/ros-csrss/win32ss/user/consrv/console.c [iso-8859-1] (original)
+++ branches/ros-csrss/win32ss/user/consrv/console.c [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -301,57 +301,51 @@
     if (ProcessData->Console != NULL)
     {
         DPRINT1("Process already has a console\n");
-        return STATUS_INVALID_PARAMETER;
-    }
-
-    RtlEnterCriticalSection(&ProcessData->HandleTableLock);
-
+        return STATUS_ACCESS_DENIED;
+    }
+
+/******************************************************************************/
+/** This comes from ConsoleConnect!!                                         **/
     DPRINT1("SrvAllocConsole - Checkpoint 1\n");
-
-    /* Initialize a new Console owned by the Console Leader Process */
-    Status = CsrInitConsole(&ProcessData->Console, AllocConsoleRequest->ShowCmd, ConsoleLeader);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Console initialization failed\n");
-        RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
-        return Status;
-    }
-
-    /* Insert the process into the processes list of the console */
-    InsertHeadList(&ProcessData->Console->ProcessList, &ProcessData->ConsoleLink);
-
-    /* Return it to the caller */
-    AllocConsoleRequest->Console = ProcessData->Console;
-
-    /* Add a reference count because the process is tied to the console */
-    _InterlockedIncrement(&ProcessData->Console->ReferenceCount);

#if 0000
     /*
-     * We've just created a new console. However when ConsoleNewProcess was
-     * called, we didn't know that we wanted to create a new console and
+     * We are about to create a new console. However when ConsoleNewProcess
+     * was called, we didn't know that we wanted to create a new console and
      * therefore, we by default inherited the handles table from our parent
      * process. It's only now that we notice that in fact we do not need
      * them, because we've created a new console and thus we must use it.
      *
-     * Therefore, free our handles table and recreate a new one.
+     * Therefore, free the console we can have and our handles table,
+     * and recreate a new one later on.
      */
-
-    ULONG i;
-
-    /* Close all console handles and free the handle table memory */
-    for (i = 0; i < ProcessData->HandleTableSize; i++)
-    {
-        Win32CsrCloseHandleEntry(&ProcessData->HandleTable[i]);
-    }
-    ProcessData->HandleTableSize = 0;
-    RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable);
-    ProcessData->HandleTable = NULL;
+    Win32CsrReleaseConsole(ProcessData);
+    // Win32CsrFreeHandlesTable(ProcessData);
 #endif
+
+    /* Initialize a new Console owned by the Console Leader Process */
+    Status = CsrInitConsole(&ProcessData->Console, AllocConsoleRequest->ShowCmd, ConsoleLeader);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("Console initialization failed\n");
+        return Status;
+    }
+
+    /* Add a reference count because the process is tied to the console */
+    _InterlockedIncrement(&ProcessData->Console->ReferenceCount);
+
+    /* Insert the process into the processes list of the console */
+    InsertHeadList(&ProcessData->Console->ProcessList, &ProcessData->ConsoleLink);
+
+    /* Return it to the caller */
+    AllocConsoleRequest->Console = ProcessData->Console;
+

/*
      * Create a new handle table - Insert the IO handles
      */
+
+    RtlEnterCriticalSection(&ProcessData->HandleTableLock);

/* Insert the Input handle */
     Status = Win32CsrInsertObject(ProcessData,
@@ -363,9 +357,10 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("Failed to insert the input handle\n");
-        ConioDeleteConsole(ProcessData->Console);
-        ProcessData->Console = NULL;
         RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+        Win32CsrReleaseConsole(ProcessData);
+        // ConioDeleteConsole(ProcessData->Console);
+        // ProcessData->Console = NULL;
         return Status;
     }

@@ -379,11 +374,12 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("Failed to insert the output handle\n");
-        ConioDeleteConsole(ProcessData->Console);
-        Win32CsrReleaseObject(ProcessData,
-                              AllocConsoleRequest->InputHandle);
-        ProcessData->Console = NULL;
         RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+        Win32CsrReleaseConsole(ProcessData);
+        // Win32CsrReleaseObject(ProcessData,
+                              // AllocConsoleRequest->InputHandle);
+        // ConioDeleteConsole(ProcessData->Console);
+        // ProcessData->Console = NULL;
         return Status;
     }

@@ -397,15 +393,18 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("Failed to insert the error handle\n");
-        ConioDeleteConsole(ProcessData->Console);
-        Win32CsrReleaseObject(ProcessData,
-                              AllocConsoleRequest->OutputHandle);
-        Win32CsrReleaseObject(ProcessData,
-                              AllocConsoleRequest->InputHandle);
-        ProcessData->Console = NULL;
         RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+        Win32CsrReleaseConsole(ProcessData);
+        // Win32CsrReleaseObject(ProcessData,
+                              // AllocConsoleRequest->OutputHandle);
+        // Win32CsrReleaseObject(ProcessData,
+                              // AllocConsoleRequest->InputHandle);
+        // ConioDeleteConsole(ProcessData->Console);
+        // ProcessData->Console = NULL;
         return Status;
     }
+
+    RtlLeaveCriticalSection(&ProcessData->HandleTableLock);

/* Duplicate the Event */
     Status = NtDuplicateObject(NtCurrentProcess(),
@@ -416,18 +415,18 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("NtDuplicateObject() failed: %lu\n", Status);
-        ConioDeleteConsole(ProcessData->Console);
-        // if (NewConsole /* || !ProcessData->bInheritHandles */)
-        {
-            Win32CsrReleaseObject(ProcessData,
-                                  AllocConsoleRequest->ErrorHandle);
-            Win32CsrReleaseObject(ProcessData,
-                                  AllocConsoleRequest->OutputHandle);
-            Win32CsrReleaseObject(ProcessData,
-                                  AllocConsoleRequest->InputHandle);
-        }
-        ProcessData->Console = NULL;
-        RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+        Win32CsrReleaseConsole(ProcessData);
+        // if (NewConsole)
+        // {
+            // Win32CsrReleaseObject(ProcessData,
+                                  // AllocConsoleRequest->ErrorHandle);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // AllocConsoleRequest->OutputHandle);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // AllocConsoleRequest->InputHandle);
+        // }
+        // ConioDeleteConsole(ProcessData->Console); // FIXME: Just release the console ?
+        // ProcessData->Console = NULL;
         return Status;
     }
     /* Input Wait Handle */
@@ -436,15 +435,149 @@
     /* Set the Ctrl Dispatcher */
     ProcessData->CtrlDispatcher = AllocConsoleRequest->CtrlDispatcher;
     DPRINT("CONSRV: CtrlDispatcher address: %x\n", ProcessData->CtrlDispatcher);
-
-    RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+/******************************************************************************/
+
     return STATUS_SUCCESS;
 }

+CSR_API(SrvAttachConsole)
+{
+    NTSTATUS Status = STATUS_SUCCESS;
+    PCONSOLE_ATTACHCONSOLE AttachConsoleRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.AttachConsoleRequest;
+    PCSR_PROCESS SourceProcess = NULL;  // The parent process.
+    PCSR_PROCESS TargetProcess = CsrGetClientThread()->Process; // Ourselves.
+    HANDLE ProcessId = ULongToHandle(AttachConsoleRequest->ProcessId);
+    PCONSOLE_PROCESS_DATA SourceProcessData, TargetProcessData;
+
+    DPRINT("SrvAttachConsole\n");
+
+    TargetProcessData = ConsoleGetPerProcessData(TargetProcess);
+
+    if (TargetProcessData->Console != NULL)
+    {
+        DPRINT1("Process already has a console\n");
+        return STATUS_ACCESS_DENIED;
+    }
+
+    if (ProcessId == ULongToHandle(ATTACH_PARENT_PROCESS))
+    {
+        PROCESS_BASIC_INFORMATION ProcessInfo;
+        ULONG Length = sizeof(ProcessInfo);
+
+        /* Get the real parent's ID */
+
+        Status = NtQueryInformationProcess(TargetProcess->ProcessHandle,
+                                           ProcessBasicInformation,
+                                           &ProcessInfo,
+                                           Length, &Length);
+        if (!NT_SUCCESS(Status))
+        {
+            DPRINT1("SrvAttachConsole - Cannot retrieve basic process info, Status = %lu\n", Status);
+            return Status;
+        }
+
+        DPRINT("We, process (ID) %lu;%lu\n", TargetProcess->ClientId.UniqueProcess, TargetProcess->ClientId.UniqueThread);
+        ProcessId = ULongToHandle(ProcessInfo.InheritedFromUniqueProcessId);
+        DPRINT("Parent process ID = %lu\n", ProcessId);
+    }
+
+    /* Lock the target process via its PID */
+    DPRINT1("Lock process Id %lu\n", ProcessId);
+    Status = CsrLockProcessByClientId(ProcessId, &SourceProcess);
+    DPRINT1("Lock process Status %lu\n", Status);
+    if (!NT_SUCCESS(Status)) return Status;
+    DPRINT1("AttachConsole OK\n");
+
+/******************************************************************************/
+/** This comes from ConsoleNewProcess!!                                      **/
+    SourceProcessData = ConsoleGetPerProcessData(SourceProcess);
+
+    /*
+     * Inherit the console from the parent,
+     * if any, otherwise return an error.
+     */
+    DPRINT1("SourceProcessData->Console = 0x%p\n", SourceProcessData->Console);
+    if (SourceProcessData->Console == NULL)
+    {
+        Status = STATUS_INVALID_HANDLE;
+        goto Quit;
+    }
+    TargetProcessData->Console = SourceProcessData->Console;
+
+    DPRINT1("SrvAttachConsole - Copy the handle table (1)\n");
+    Status = Win32CsrInheritHandlesTable(SourceProcessData, TargetProcessData);
+    DPRINT1("SrvAttachConsole - Copy the handle table (2)\n");
+    if (!NT_SUCCESS(Status))
+    {
+        goto Quit;
+    }
+
+/******************************************************************************/
+
+/******************************************************************************/
+/** This comes from ConsoleConnect / SrvAllocConsole!!                       **/
+    /* Add a reference count because the process is tied to the console */
+    _InterlockedIncrement(&TargetProcessData->Console->ReferenceCount);
+
+    /* Insert the process into the processes list of the console */
+    InsertHeadList(&TargetProcessData->Console->ProcessList, &TargetProcessData->ConsoleLink);
+
+    /* Return it to the caller */
+    AttachConsoleRequest->Console = TargetProcessData->Console;
+
+    /** Here, we inherited the console handles from the "source" process,
+     ** so no need to reinitialize the handles table. **/
+
+    DPRINT1("SrvAttachConsole - Checkpoint\n");
+
+    /* Duplicate the Event */
+    Status = NtDuplicateObject(NtCurrentProcess(),
+                               TargetProcessData->Console->ActiveEvent,
+                               TargetProcessData->Process->ProcessHandle,
+                               &TargetProcessData->ConsoleEvent,
+                               EVENT_ALL_ACCESS, 0, 0);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("NtDuplicateObject() failed: %lu\n", Status);
+        Win32CsrReleaseConsole(TargetProcessData);
+// #if 0
+        // if (NewConsole)
+        // {
+            // Win32CsrReleaseObject(TargetProcessData,
+                                  // AttachConsoleRequest->ErrorHandle);
+            // Win32CsrReleaseObject(TargetProcessData,
+                                  // AttachConsoleRequest->OutputHandle);
+            // Win32CsrReleaseObject(TargetProcessData,
+                                  // AttachConsoleRequest->InputHandle);
+        // }
+// #endif
+        // ConioDeleteConsole(TargetProcessData->Console); // FIXME: Just release the console ?
+        // TargetProcessData->Console = NULL;
+        goto Quit;
+    }
+    /* Input Wait Handle */
+    AttachConsoleRequest->InputWaitHandle = TargetProcessData->ConsoleEvent;
+
+    /* Set the Ctrl Dispatcher */
+    TargetProcessData->CtrlDispatcher = AttachConsoleRequest->CtrlDispatcher;
+    DPRINT("CONSRV: CtrlDispatcher address: %x\n", TargetProcessData->CtrlDispatcher);
+
+    Status = STATUS_SUCCESS;
+/******************************************************************************/
+
+Quit:
+    DPRINT1("SrvAttachConsole - exiting 1\n");
+    /* Unlock the "source" process */
+    CsrUnlockProcess(SourceProcess);
+    DPRINT1("SrvAttachConsole - exiting 2\n");
+
+    return Status;
+}
+
 CSR_API(SrvFreeConsole)
 {
     DPRINT1("SrvFreeConsole\n");
-    Win32CsrReleaseConsole(CsrGetClientThread()->Process);
+    Win32CsrReleaseConsole(ConsoleGetPerProcessData(CsrGetClientThread()->Process));
     return STATUS_SUCCESS;
 }

Modified: branches/ros-csrss/win32ss/user/consrv/consrv.h
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/win32ss/user/consrv/co...
==============================================================================
--- branches/ros-csrss/win32ss/user/consrv/consrv.h [iso-8859-1] (original)
+++ branches/ros-csrss/win32ss/user/consrv/consrv.h [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -73,7 +73,6 @@
     /* PCONSOLE */ struct _CONSOLE* Console;
     /* PCONSOLE */ struct _CONSOLE* ParentConsole;

-    // BOOL bInheritHandles;
     BOOL ConsoleApp;    // TRUE if it is a CUI app, FALSE otherwise.

RTL_CRITICAL_SECTION HandleTableLock;
@@ -118,6 +117,7 @@
 /* console.c */
 CSR_API(SrvOpenConsole);
 CSR_API(SrvAllocConsole);
+CSR_API(SrvAttachConsole);
 CSR_API(SrvFreeConsole);
 CSR_API(SrvSetConsoleMode);
 CSR_API(SrvGetConsoleMode);
@@ -139,6 +139,9 @@
 CSR_API(SrvDuplicateHandle);
 /// CSR_API(CsrGetInputWaitHandle);

+NTSTATUS FASTCALL Win32CsrInheritHandlesTable(IN PCONSOLE_PROCESS_DATA SourceProcessData,
+                                              IN PCONSOLE_PROCESS_DATA TargetProcessData);
+VOID FASTCALL Win32CsrFreeHandlesTable(PCONSOLE_PROCESS_DATA ProcessData);
 NTSTATUS FASTCALL Win32CsrInsertObject(PCONSOLE_PROCESS_DATA ProcessData,
                                        PHANDLE Handle,
                                        Object_t *Object,
@@ -153,6 +156,7 @@
 VOID FASTCALL Win32CsrUnlockObject(Object_t *Object);
 NTSTATUS FASTCALL Win32CsrReleaseObject(PCONSOLE_PROCESS_DATA ProcessData,
                                         HANDLE Handle);
+VOID FASTCALL Win32CsrReleaseConsole(PCONSOLE_PROCESS_DATA ProcessData);

NTSTATUS NTAPI ConsoleNewProcess(PCSR_PROCESS SourceProcess,
                                  PCSR_PROCESS TargetProcess);
@@ -160,7 +164,6 @@
                               IN OUT PVOID ConnectionInfo,
                               IN OUT PULONG ConnectionInfoLength);
 VOID NTAPI ConsoleDisconnect(PCSR_PROCESS Process);
-VOID NTAPI Win32CsrReleaseConsole(PCSR_PROCESS Process);

/* lineinput.c */
 CSR_API(SrvGetConsoleCommandHistoryLength);

Modified: branches/ros-csrss/win32ss/user/consrv/handle.c
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/win32ss/user/consrv/ha...
==============================================================================
--- branches/ros-csrss/win32ss/user/consrv/handle.c [iso-8859-1] (original)
+++ branches/ros-csrss/win32ss/user/consrv/handle.c [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -81,6 +81,85 @@

NTSTATUS
 FASTCALL
+Win32CsrInheritHandlesTable(IN PCONSOLE_PROCESS_DATA SourceProcessData,
+                            IN PCONSOLE_PROCESS_DATA TargetProcessData)
+{
+    NTSTATUS Status = STATUS_SUCCESS;
+    ULONG i;
+
+    RtlEnterCriticalSection(&SourceProcessData->HandleTableLock);
+
+    /* Inherit a handles table only if there is no already */
+    if (TargetProcessData->HandleTable != NULL /* || TargetProcessData->HandleTableSize != 0 */)
+    {
+        Status = STATUS_UNSUCCESSFUL; /* STATUS_INVALID_PARAMETER */
+        goto Quit;
+    }
+
+    /* Allocate a new handle table for the child process */
+    TargetProcessData->HandleTable = RtlAllocateHeap(ConSrvHeap,
+                                                     HEAP_ZERO_MEMORY,
+                                                     SourceProcessData->HandleTableSize
+                                                             * sizeof(CONSOLE_IO_HANDLE));
+    if (TargetProcessData->HandleTable == NULL)
+    {
+        Status = STATUS_NO_MEMORY;
+        goto Quit;
+    }
+
+    TargetProcessData->HandleTableSize = SourceProcessData->HandleTableSize;
+
+    /*
+     * Parse the parent process' handles table and, for each handle,
+     * do a copy of it and reference it, if the handle is inheritable.
+     */
+    for (i = 0; i < SourceProcessData->HandleTableSize; i++)
+    {
+        if (SourceProcessData->HandleTable[i].Object != NULL &&
+            SourceProcessData->HandleTable[i].Inheritable)
+        {
+            /*
+             * Copy the handle data and increment the reference count of the
+             * pointed object (via the call to Win32CsrCreateHandleEntry).
+             */
+            TargetProcessData->HandleTable[i] = SourceProcessData->HandleTable[i];
+            Win32CsrCreateHandleEntry(&TargetProcessData->HandleTable[i]);
+        }
+    }
+
+Quit:
+    RtlLeaveCriticalSection(&SourceProcessData->HandleTableLock);
+    return Status;
+}
+
+VOID
+FASTCALL
+Win32CsrFreeHandlesTable(PCONSOLE_PROCESS_DATA ProcessData)
+{
+    DPRINT1("Win32CsrFreeHandlesTable\n");
+
+    RtlEnterCriticalSection(&ProcessData->HandleTableLock);
+
+    if (ProcessData->HandleTable != NULL)
+    {
+        ULONG i;
+
+        /* Close all console handles and free the handle table memory */
+        for (i = 0; i < ProcessData->HandleTableSize; i++)
+        {
+            Win32CsrCloseHandleEntry(&ProcessData->HandleTable[i]);
+        }
+        RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable);
+        ProcessData->HandleTable = NULL;
+    }
+
+    ProcessData->HandleTableSize = 0;
+
+    RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+}
+
+NTSTATUS
+FASTCALL
 Win32CsrInsertObject(PCONSOLE_PROCESS_DATA ProcessData,
                      PHANDLE Handle,
                      Object_t *Object,
@@ -122,7 +201,8 @@
     ProcessData->HandleTable[i].Inheritable = Inheritable;
     ProcessData->HandleTable[i].ShareMode   = ShareMode;
     Win32CsrCreateHandleEntry(&ProcessData->HandleTable[i]);
-    *Handle = UlongToHandle((i << 2) | 0x3);
+    *Handle = ULongToHandle((i << 2) | 0x3);
+
     RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
     return STATUS_SUCCESS;
 }
@@ -202,9 +282,33 @@
     Win32CsrUnlockConsole(Object->Console);
 }

-
-
-/** Remark: this function can be called by SrvAttachConsole (not yet implemented) **/
+VOID
+FASTCALL
+Win32CsrReleaseConsole(PCONSOLE_PROCESS_DATA ProcessData)
+{
+    PCONSOLE Console;
+
+    DPRINT1("Win32CsrReleaseConsole\n");
+
+    /* Close all console handles and free the handle table memory */
+    Win32CsrFreeHandlesTable(ProcessData);
+
+    /* Detach process from console */
+    Console = ProcessData->Console;
+    if (Console != NULL)
+    {
+        DPRINT1("Win32CsrReleaseConsole - Console->ReferenceCount = %lu - We are going to decrement it !\n", Console->ReferenceCount);
+        ProcessData->Console = NULL;
+        EnterCriticalSection(&Console->Lock);
+        RemoveEntryList(&ProcessData->ConsoleLink);
+        Win32CsrUnlockConsole(Console);
+        //CloseHandle(ProcessData->ConsoleEvent);
+        //ProcessData->ConsoleEvent = NULL;
+    }
+}
+
+
+
 NTSTATUS
 NTAPI
 ConsoleNewProcess(PCSR_PROCESS SourceProcess,
@@ -223,7 +327,6 @@
      **************************************************************************/

PCONSOLE_PROCESS_DATA SourceProcessData, TargetProcessData;
-    ULONG i;

DPRINT1("ConsoleNewProcess inside\n");
     DPRINT1("SourceProcess = 0x%p ; TargetProcess = 0x%p\n", SourceProcess, TargetProcess);
@@ -243,14 +346,13 @@
     TargetProcessData->Process = TargetProcess;
     TargetProcessData->ConsoleEvent = NULL;
     TargetProcessData->Console = TargetProcessData->ParentConsole = NULL;
-    // TargetProcessData->bInheritHandles = FALSE;
     TargetProcessData->ConsoleApp = ((TargetProcess->Flags & CsrProcessIsConsoleApp) ? TRUE : FALSE);

// Testing
     TargetProcessData->HandleTableSize = 0;
     TargetProcessData->HandleTable = NULL;

-    /* HACK */ RtlZeroMemory(&TargetProcessData->HandleTableLock, sizeof(RTL_CRITICAL_SECTION));
+    /**** HACK !!!! ****/ RtlZeroMemory(&TargetProcessData->HandleTableLock, sizeof(RTL_CRITICAL_SECTION));
     RtlInitializeCriticalSection(&TargetProcessData->HandleTableLock);

/* Do nothing if the source process is NULL */
@@ -267,51 +369,17 @@
     if ( SourceProcessData->Console != NULL && /* SourceProcessData->ConsoleApp */
          TargetProcessData->ConsoleApp )
     {
-/*
-        if (TargetProcessData->HandleTableSize)
-        {
-            return STATUS_INVALID_PARAMETER;
-        }
-*/
-
-        DPRINT1("ConsoleNewProcess - Copy the handle table (1)\n");
+        NTSTATUS Status;
+
+        Status = Win32CsrInheritHandlesTable(SourceProcessData, TargetProcessData);
+        if (!NT_SUCCESS(Status))
+        {
+            return Status;
+        }
+
+        // FIXME: Do it before, or after the handles table inheritance ??
         /* Temporary "inherit" the console from the parent */
         TargetProcessData->ParentConsole = SourceProcessData->Console;
-        RtlEnterCriticalSection(&SourceProcessData->HandleTableLock);
-        DPRINT1("ConsoleNewProcess - Copy the handle table (2)\n");
-
-        /* Allocate a new handle table for the child process */
-        TargetProcessData->HandleTable = RtlAllocateHeap(ConSrvHeap,
-                                                         HEAP_ZERO_MEMORY,
-                                                         SourceProcessData->HandleTableSize
-                                                                 * sizeof(CONSOLE_IO_HANDLE));
-        if (TargetProcessData->HandleTable == NULL)
-        {
-            RtlLeaveCriticalSection(&SourceProcessData->HandleTableLock);
-            return STATUS_UNSUCCESSFUL;
-        }
-
-        TargetProcessData->HandleTableSize = SourceProcessData->HandleTableSize;
-
-        /*
-         * Parse the parent process' handles table and, for each handle,
-         * do a copy of it and reference it, if the handle is inheritable.
-         */
-        for (i = 0; i < SourceProcessData->HandleTableSize; i++)
-        {
-            if (SourceProcessData->HandleTable[i].Object != NULL &&
-                SourceProcessData->HandleTable[i].Inheritable)
-            {
-                /*
-                 * Copy the handle data and increment the reference count of the
-                 * pointed object (via the call to Win32CsrCreateHandleEntry).
-                 */
-                TargetProcessData->HandleTable[i] = SourceProcessData->HandleTable[i];
-                Win32CsrCreateHandleEntry(&TargetProcessData->HandleTable[i]);
-            }
-        }
-
-        RtlLeaveCriticalSection(&SourceProcessData->HandleTableLock);
     }
     else
     {
@@ -335,7 +403,6 @@
     PCONSOLE_CONNECTION_INFO ConnectInfo = (PCONSOLE_CONNECTION_INFO)ConnectionInfo;
     PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(CsrProcess);
     BOOLEAN NewConsole = FALSE;
-    // PCONSOLE Console = NULL;

DPRINT1("ConsoleConnect\n");

@@ -354,13 +421,23 @@
         return STATUS_SUCCESS;
     }

-    RtlEnterCriticalSection(&ProcessData->HandleTableLock);
-
     /* If we don't have a console, then create a new one... */
     if (!ConnectInfo->Console ||
          ConnectInfo->Console != ProcessData->ParentConsole)
     {
         DPRINT1("ConsoleConnect - Allocate a new console\n");
+
+        /*
+         * We are about to create a new console. However when ConsoleNewProcess
+         * was called, we didn't know that we wanted to create a new console and
+         * therefore, we by default inherited the handles table from our parent
+         * process. It's only now that we notice that in fact we do not need
+         * them, because we've created a new console and thus we must use it.
+         *
+         * Therefore, free the console we can have and our handles table,
+         * and recreate a new one later on.
+         */
+        Win32CsrReleaseConsole(ProcessData);

/* Initialize a new Console owned by the Console Leader Process */
         NewConsole = TRUE;
@@ -368,7 +445,6 @@
         if (!NT_SUCCESS(Status))
         {
             DPRINT1("Console initialization failed\n");
-            RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
             return Status;
         }
     }
@@ -381,41 +457,22 @@
         ProcessData->Console = ConnectInfo->Console;
     }

+    /* Add a reference count because the process is tied to the console */
+    _InterlockedIncrement(&ProcessData->Console->ReferenceCount);
+
     /* Insert the process into the processes list of the console */
     InsertHeadList(&ProcessData->Console->ProcessList, &ProcessData->ConsoleLink);

/* Return it to the caller */
     ConnectInfo->Console = ProcessData->Console;

-    /* Add a reference count because the process is tied to the console */
-    _InterlockedIncrement(&ProcessData->Console->ReferenceCount);
-
-    if (NewConsole /* || !ProcessData->bInheritHandles */)
-    {
-        /*
-         * We've just created a new console. However when ConsoleNewProcess was
-         * called, we didn't know that we wanted to create a new console and
-         * therefore, we by default inherited the handles table from our parent
-         * process. It's only now that we notice that in fact we do not need
-         * them, because we've created a new console and thus we must use it.
-         *
-         * Therefore, free our handles table and recreate a new one.
-         */
-
-        ULONG i;
-
-        /* Close all console handles and free the handle table memory */
-        for (i = 0; i < ProcessData->HandleTableSize; i++)
-        {
-            Win32CsrCloseHandleEntry(&ProcessData->HandleTable[i]);
-        }
-        ProcessData->HandleTableSize = 0;
-        RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable);
-        ProcessData->HandleTable = NULL;
-
+    if (NewConsole)
+    {
         /*
          * Create a new handle table - Insert the IO handles
          */
+
+        RtlEnterCriticalSection(&ProcessData->HandleTableLock);

/* Insert the Input handle */
         Status = Win32CsrInsertObject(ProcessData,
@@ -427,9 +484,10 @@
         if (!NT_SUCCESS(Status))
         {
             DPRINT1("Failed to insert the input handle\n");
-            ConioDeleteConsole(ProcessData->Console);
-            ProcessData->Console = NULL;
             RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+            Win32CsrReleaseConsole(ProcessData);
+            // ConioDeleteConsole(ProcessData->Console);
+            // ProcessData->Console = NULL;
             return Status;
         }

@@ -443,11 +501,12 @@
         if (!NT_SUCCESS(Status))
         {
             DPRINT1("Failed to insert the output handle\n");
-            ConioDeleteConsole(ProcessData->Console);
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->InputHandle);
-            ProcessData->Console = NULL;
             RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+            Win32CsrReleaseConsole(ProcessData);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->InputHandle);
+            // ConioDeleteConsole(ProcessData->Console);
+            // ProcessData->Console = NULL;
             return Status;
         }

@@ -461,15 +520,18 @@
         if (!NT_SUCCESS(Status))
         {
             DPRINT1("Failed to insert the error handle\n");
-            ConioDeleteConsole(ProcessData->Console);
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->OutputHandle);
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->InputHandle);
-            ProcessData->Console = NULL;
             RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+            Win32CsrReleaseConsole(ProcessData);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->OutputHandle);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->InputHandle);
+            // ConioDeleteConsole(ProcessData->Console);
+            // ProcessData->Console = NULL;
             return Status;
         }
+
+        RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
     }

/* Duplicate the Event */
@@ -481,18 +543,18 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("NtDuplicateObject() failed: %lu\n", Status);
-        ConioDeleteConsole(ProcessData->Console);
-        if (NewConsole /* || !ProcessData->bInheritHandles */)
-        {
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->ErrorHandle);
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->OutputHandle);
-            Win32CsrReleaseObject(ProcessData,
-                                  ConnectInfo->InputHandle);
-        }
-        ProcessData->Console = NULL;
-        RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+        Win32CsrReleaseConsole(ProcessData);
+        // if (NewConsole)
+        // {
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->ErrorHandle);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->OutputHandle);
+            // Win32CsrReleaseObject(ProcessData,
+                                  // ConnectInfo->InputHandle);
+        // }
+        // ConioDeleteConsole(ProcessData->Console); // FIXME: Just release the console ?
+        // ProcessData->Console = NULL;
         return Status;
     }
     /* Input Wait Handle */
@@ -500,47 +562,9 @@

/* Set the Ctrl Dispatcher */
     ProcessData->CtrlDispatcher = ConnectInfo->CtrlDispatcher;
-    DPRINT("CSRSS:CtrlDispatcher address: %x\n", ProcessData->CtrlDispatcher);
-
-    RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
+    DPRINT("CONSRV: CtrlDispatcher address: %x\n", ProcessData->CtrlDispatcher);
+
     return STATUS_SUCCESS;
-}
-
-VOID
-WINAPI
-Win32CsrReleaseConsole(PCSR_PROCESS Process)
-{
-    PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(Process);
-    PCONSOLE Console;
-    ULONG i;
-
-    DPRINT1("Win32CsrReleaseConsole\n");
-
-    RtlEnterCriticalSection(&ProcessData->HandleTableLock);
-
-    /* Close all console handles and free the handle table memory */
-    for (i = 0; i < ProcessData->HandleTableSize; i++)
-    {
-        Win32CsrCloseHandleEntry(&ProcessData->HandleTable[i]);
-    }
-    ProcessData->HandleTableSize = 0;
-    RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable);
-    ProcessData->HandleTable = NULL;
-
-    /* Detach process from console */
-    Console = ProcessData->Console;
-    if (Console != NULL)
-    {
-        DPRINT1("Win32CsrReleaseConsole - Console->ReferenceCount = %lu - We are going to decrement it !\n", Console->ReferenceCount);
-        ProcessData->Console = NULL;
-        EnterCriticalSection(&Console->Lock);
-        RemoveEntryList(&ProcessData->ConsoleLink);
-        Win32CsrUnlockConsole(Console);
-        //CloseHandle(ProcessData->ConsoleEvent);
-        //ProcessData->ConsoleEvent = NULL;
-    }
-
-    RtlLeaveCriticalSection(&ProcessData->HandleTableLock);
 }

VOID
@@ -551,19 +575,14 @@

/**************************************************************************
      * This function is called whenever a new process (GUI or CUI) is destroyed.
-     *
-     * Only do something if the process is a CUI. <-- modify this behaviour if
-     *                                                we deal with a GUI which
-     *                                                quits and acquired a
-     *                                                console...
      **************************************************************************/

DPRINT1("ConsoleDisconnect called\n");
-    // if (ProcessData->Console != NULL)
-    if (ProcessData->ConsoleApp)
+    if ( ProcessData->Console     != NULL ||
+         ProcessData->HandleTable != NULL )
     {
         DPRINT1("ConsoleDisconnect - calling Win32CsrReleaseConsole\n");
-        Win32CsrReleaseConsole(Process);
+        Win32CsrReleaseConsole(ProcessData);
     }

RtlDeleteCriticalSection(&ProcessData->HandleTableLock);

Modified: branches/ros-csrss/win32ss/user/consrv/init.c
URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/win32ss/user/consrv/in...
==============================================================================
--- branches/ros-csrss/win32ss/user/consrv/init.c [iso-8859-1] (original)
+++ branches/ros-csrss/win32ss/user/consrv/init.c [iso-8859-1] Sun Jan 13 17:07:25 2013
@@ -107,7 +107,7 @@
     // SrvRegisterConsoleIME,
     // SrvUnregisterConsoleIME,
     // SrvGetConsoleLangId,
-    // SrvAttachConsole,
+    SrvAttachConsole,
     SrvGetConsoleSelectionInfo,
     SrvGetConsoleProcessList,
     SrvGetConsoleHistory,
@@ -198,7 +198,7 @@
     // FALSE,   // SrvRegisterConsoleIME,
     // FALSE,   // SrvUnregisterConsoleIME,
     // FALSE,   // SrvGetConsoleLangId,
-    // FALSE,   // SrvAttachConsole,
+    FALSE,   // SrvAttachConsole,
     FALSE,   // SrvGetConsoleSelectionInfo,
     FALSE,   // SrvGetConsoleProcessList,
     FALSE,   // SrvGetConsoleHistory,
@@ -291,7 +291,7 @@
     // "RegisterConsoleIME",
     // "UnregisterConsoleIME",
     // "GetConsoleLangId",
-    // "AttachConsole",
+    "AttachConsole",
     "GetConsoleSelectionInfo",
     "GetConsoleProcessList",
     "GetConsoleHistory",