Merge pull request #1175 from mikem8361/palinit2

Some more pal init cleanup missed in #1174

4 files changed, 86 insertions, 65 deletions

diff --git a/src/pal/src/debug/debug.cpp b/src/pal/src/debug/debug.cpp
index 1e108eef4f..dc0c2be248 100644
--- a/src/pal/src/debug/debug.cpp
+++ b/src/pal/src/debug/debug.cpp
@@ -339,6 +339,7 @@ int
 DebugBreakCommand()
 {
 #ifdef ENABLE_RUN_ON_DEBUG_BREAK
+    extern MODSTRUCT exe_module;
     const char *command_string = getenv (PAL_RUN_ON_DEBUG_BREAK);
     if (command_string) {
         char pid_buf[sizeof (PID_TEXT) + 32];
diff --git a/src/pal/src/include/pal/module.h b/src/pal/src/include/pal/module.h
index e0986de8e0..ce3eaa978f 100644
--- a/src/pal/src/include/pal/module.h
+++ b/src/pal/src/include/pal/module.h
@@ -64,14 +64,12 @@ typedef struct _MODSTRUCT
     struct _MODSTRUCT *prev;
 } MODSTRUCT;
 
-extern MODSTRUCT exe_module;
 
 /*++
 Function :
     LOADInitializeModules
 
-    Initialize the process-wide list of modules (2 initial modules : 1 for
-    the executable and 1 for the PAL)
+    Initialize the process-wide list of modules
 
 Parameters :
     None
@@ -84,6 +82,22 @@ BOOL LOADInitializeModules();
 
 /*++
 Function :
+    LOADSetExeName
+
+    Set the exe name path
+
+Parameters :
+    LPWSTR man exe path and name
+
+Return value :
+    TRUE  if initialization succeedded
+    FALSE otherwise
+
+--*/
+BOOL LOADSetExeName(LPWSTR name);
+
+/*++
+Function :
     LOADFreeModules
 
     Release all resources held by the module manager (including dlopen handles)
diff --git a/src/pal/src/init/pal.cpp b/src/pal/src/init/pal.cpp
index c2e66a5d3e..882c9e2bb7 100644
--- a/src/pal/src/init/pal.cpp
+++ b/src/pal/src/init/pal.cpp
@@ -233,7 +233,6 @@ Initialize(
         gPID = getpid();
 
         fFirstTimeInit = true;
-        exe_module.lib_name = NULL;
 
         // Initialize the TLS lookaside cache
         if (FALSE == TLSInitialize())
@@ -344,6 +343,16 @@ Initialize(
         g_fThreadDataAvailable = TRUE;
 
         //
+        // Initialize module manager
+        //
+        if (FALSE == LOADInitializeModules())
+        {
+            ERROR("Unable to initialize module manager\n");
+            palError = ERROR_INTERNAL_ERROR;
+            goto CLEANUP1b;
+        }
+
+        //
         // Initialize the object manager
         //
 
@@ -423,22 +432,27 @@ Initialize(
         // InitializeProcessCommandLine took ownership of this memory.
         command_line = NULL;
 
-        // Save the exe path in the exe module struct
-        InternalFree(pThread, exe_module.lib_name);
-        exe_module.lib_name = exe_path;
-
 #ifdef PAL_PERF
         // Initialize the Profiling structure
         if(FALSE == PERFInitialize(command_line, exe_path)) 
         {
             ERROR("Performance profiling initial failed\n");
-            goto done;
+            goto CLEANUP2;
         }    
         PERFAllocThreadInfo();
 #endif
+
+        if (!LOADSetExeName(exe_path))
+        {
+            ERROR("Unable to set exe name\n");
+            goto CLEANUP2;
+        }
+
+        // LOADSetExeName took ownership of this memory.
+        exe_path = NULL;
     }
 
-    if(init_count == 0)
+    if (init_count == 0)
     {
         //
         // Create the initial process and thread objects
@@ -479,14 +493,6 @@ Initialize(
             goto CLEANUP6;
         }
 
-        /* Initialize module manager */
-        if (FALSE == LOADInitializeModules())
-        {
-            ERROR("Unable to initialize module manager\n");
-            palError = GetLastError();
-            goto CLEANUP8;
-        }
-         
         /* Initialize the Virtual* functions. */
         if (FALSE == VIRTUALInitialize())
         {
@@ -513,7 +519,6 @@ Initialize(
         /* Set LastError to a non-good value - functions within the
            PAL startup may set lasterror to a nonzero value. */
         SetLastError(NO_ERROR);
-        
         retval = 0;
     }
     else
@@ -540,7 +545,6 @@ CLEANUP13:
     VIRTUALCleanup();
 CLEANUP10:
     LOADFreeModules(TRUE);
-CLEANUP8:
     MAPCleanup();
 CLEANUP6:
     SEHCleanup();
@@ -549,10 +553,7 @@ CLEANUP5:
 CLEANUP2:
     InternalFree(pThread, exe_path);
 CLEANUP1e:
-    if (command_line != NULL)
-    {
-        InternalFree(pThread, command_line);
-    }
+    InternalFree(pThread, command_line);
 CLEANUP1d:
     // Cleanup synchronization manager
 CLEANUP1c:
diff --git a/src/pal/src/loader/module.cpp b/src/pal/src/loader/module.cpp
index 2a7d2c4b54..1e7c2245a0 100644
--- a/src/pal/src/loader/module.cpp
+++ b/src/pal/src/loader/module.cpp
@@ -88,7 +88,8 @@ SET_DEFAULT_DEBUG_CHANNEL(LOADER);
 /* critical section that regulates access to the module list */
 CRITICAL_SECTION module_critsec;
 
-MODSTRUCT exe_module; /* always the first, in the in-load-order list */
+/* always the first, in the in-load-order list */
+MODSTRUCT exe_module; 
 MODSTRUCT *pal_module = NULL;
 
 char g_szCoreCLRPath[MAX_PATH] = { 0 };
@@ -836,8 +837,7 @@ PAL_UnregisterLibraryW(
 Function :
     LOADInitializeModules
 
-    Initialize the process-wide list of modules (2 initial modules : 1 for
-    the executable and 1 for the PAL)
+    Initialize the process-wide list of modules
 
 Parameters :
     None
@@ -850,18 +850,7 @@ Return value :
 extern "C"
 BOOL LOADInitializeModules()
 {
-#if RETURNS_NEW_HANDLES_ON_REPEAT_DLOPEN
-    LPSTR pszExeName = NULL;
-#endif
-
-    BOOL fRetCode = FALSE;
-    LPWSTR lpwstr = NULL;
-
-    if (exe_module.prev)
-    {
-        ERROR("Module manager already initialized!\n");
-        goto exit;
-    }
+    _ASSERTE(exe_module.prev == NULL);
 
     InternalInitializeCriticalSection(&module_critsec);
 
@@ -870,67 +859,83 @@ BOOL LOADInitializeModules()
 
     exe_module.self = (HMODULE)&exe_module;
     exe_module.dl_handle = dlopen(NULL, RTLD_LAZY);
-    if (!exe_module.dl_handle)
+    if (exe_module.dl_handle == NULL)
     {
-        ERROR("Main executable module will be broken : dlopen(NULL) failed"
-            "dlerror message is \"%s\" \n", dlerror());
-        goto exit;
+        ERROR("Executable module will be broken : dlopen(NULL) failed dlerror message is \"%s\" \n", dlerror());
+        return FALSE;
     }
+    exe_module.lib_name = NULL;
     exe_module.refcount = -1;
     exe_module.next = &exe_module;
     exe_module.prev = &exe_module;
     exe_module.pDllMain = NULL;
     exe_module.hinstance = NULL;
     exe_module.threadLibCalls = TRUE;
+    return TRUE;
+}
+
+/*++
+Function :
+    LOADSetExeName
+
+    Set the exe name path
+
+Parameters :
+    LPWSTR man exe path and name
+
+Return value :
+    TRUE  if initialization succeedded
+    FALSE otherwise
+
+--*/
+extern "C"
+BOOL LOADSetExeName(LPWSTR name)
+{
+    CPalThread *pThread = InternalGetCurrentThread();
+#if RETURNS_NEW_HANDLES_ON_REPEAT_DLOPEN
+    LPSTR pszExeName = NULL;
+#endif
+    BOOL result = FALSE;
+
+    LockModuleList();
+
+    // Save the exe path in the exe module struct
+    InternalFree(pThread, exe_module.lib_name);
+    exe_module.lib_name = name;
 
     // For platforms where we can't trust the handle to be constant, we need to 
     // store the inode/device pairs for the modules we just initialized.
 #if RETURNS_NEW_HANDLES_ON_REPEAT_DLOPEN
     {
         struct stat stat_buf;
-        pszExeName = UTIL_WCToMB_Alloc(exe_module.lib_name, -1);
+        pszExeName = UTIL_WCToMB_Alloc(name, -1);
         if (NULL == pszExeName)
         {
             ERROR("WCToMB failure, unable to get full name of exe\n");
             goto exit;
         }
-        if ( -1 == stat(pszExeName, &stat_buf))
+        if (-1 == stat(pszExeName, &stat_buf))
         {
             SetLastError(ERROR_MOD_NOT_FOUND);
             goto exit;
         }
-
-        TRACE("Executable has inode %d and device %d\n", 
-            stat_buf.st_ino, stat_buf.st_dev);
+        TRACE("Executable has inode %d and device %d\n", stat_buf.st_ino, stat_buf.st_dev);
 
         exe_module.inode = stat_buf.st_ino; 
         exe_module.device = stat_buf.st_dev;
     }
 #endif
-
-    // If we got here, init succeeded.
-    fRetCode = TRUE;
-
-exit:
-    CPalThread *pThread = InternalGetCurrentThread();
-    if (!fRetCode)
-    {
-        InternalFree(pThread, lpwstr);
-        if (GetLastError() == ERROR_SUCCESS)
-        {
-            SetLastError(ERROR_INTERNAL_ERROR);
-        }
-    }
+    result = TRUE;
 
 #if RETURNS_NEW_HANDLES_ON_REPEAT_DLOPEN
+exit:
     if (pszExeName)
     {
         InternalFree(pThread, pszExeName);
     }
-
 #endif
-    TRACE("Module manager initialization returning %d.\n", fRetCode);
-    return fRetCode;
+    UnlockModuleList();
+    return result;
 }
 
 /*++