[PATCH v2] msvcrt: Use already existent oneexit_table functions for _onexit and _c_exit

[Fabian Maurer]

For that to work, MSVCRT__register_onexit_function and
MSVCRT__execute_onexit_table have to available all the time,
not only when _MSVCR_VER>=140

tests based on code by Piotr Caban

v2:
Create exit_event2 in test function, so it exists in child process

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
---
 dlls/msvcrt/exit.c         | 68 ++++++++++-------------------------
 dlls/ucrtbase/tests/misc.c | 74 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 51 deletions(-)

diff --git a/dlls/msvcrt/exit.c b/dlls/msvcrt/exit.c
index 7e1805569c..c90f7df953 100644
--- a/dlls/msvcrt/exit.c
+++ b/dlls/msvcrt/exit.c
@@ -25,13 +25,6 @@
 
 WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
 
-/* MT */
-#define LOCK_EXIT   _mlock(_EXIT_LOCK1)
-#define UNLOCK_EXIT _munlock(_EXIT_LOCK1)
-
-static MSVCRT__onexit_t *MSVCRT_atexit_table = NULL;
-static int MSVCRT_atexit_table_size = 0;
-static int MSVCRT_atexit_registered = 0; /* Points to free slot */
 static MSVCRT_purecall_handler purecall_handler = NULL;
 
 typedef struct MSVCRT__onexit_table_t
@@ -41,6 +34,11 @@ typedef struct MSVCRT__onexit_table_t
     MSVCRT__onexit_t *_end;
 } MSVCRT__onexit_table_t;
 
+static MSVCRT__onexit_table_t MSVCRT_atexit_table;
+
+int CDECL MSVCRT__register_onexit_function(MSVCRT__onexit_table_t *table, MSVCRT__onexit_t func);
+int CDECL MSVCRT__execute_onexit_table(MSVCRT__onexit_table_t *table);
+
 typedef void (__stdcall *_tls_callback_type)(void*,ULONG,void*);
 static _tls_callback_type tls_atexit_callback;
 
@@ -65,17 +63,8 @@ void (*CDECL _aexit_rtn)(int) = MSVCRT__exit;
 static void __MSVCRT__call_atexit(void)
 {
   /* Note: should only be called with the exit lock held */
-  TRACE("%d atext functions to call\n", MSVCRT_atexit_registered);
   if (tls_atexit_callback) tls_atexit_callback(NULL, DLL_PROCESS_DETACH, NULL);
-  /* Last registered gets executed first */
-  while (MSVCRT_atexit_registered > 0)
-  {
-    MSVCRT_atexit_registered--;
-    TRACE("next is %p\n",MSVCRT_atexit_table[MSVCRT_atexit_registered]);
-    if (MSVCRT_atexit_table[MSVCRT_atexit_registered])
-      (*MSVCRT_atexit_table[MSVCRT_atexit_registered])();
-    TRACE("returned\n");
-  }
+  MSVCRT__execute_onexit_table(&MSVCRT_atexit_table);
 }
 
 /*********************************************************************
@@ -276,9 +265,7 @@ void CDECL MSVCRT__c_exit(void)
 void CDECL MSVCRT__cexit(void)
 {
   TRACE("(void)\n");
-  LOCK_EXIT;
   __MSVCRT__call_atexit();
-  UNLOCK_EXIT;
 }
 
 /*********************************************************************
@@ -291,26 +278,8 @@ MSVCRT__onexit_t CDECL MSVCRT__onexit(MSVCRT__onexit_t func)
   if (!func)
     return NULL;
 
-  LOCK_EXIT;
-  if (MSVCRT_atexit_registered > MSVCRT_atexit_table_size - 1)
-  {
-    MSVCRT__onexit_t *newtable;
-    TRACE("expanding table\n");
-    newtable = MSVCRT_calloc(MSVCRT_atexit_table_size + 32, sizeof(void *));
-    if (!newtable)
-    {
-      TRACE("failed!\n");
-      UNLOCK_EXIT;
-      return NULL;
-    }
-    memcpy (newtable, MSVCRT_atexit_table, MSVCRT_atexit_table_size*sizeof(void *));
-    MSVCRT_atexit_table_size += 32;
-    MSVCRT_free (MSVCRT_atexit_table);
-    MSVCRT_atexit_table = newtable;
-  }
-  MSVCRT_atexit_table[MSVCRT_atexit_registered] = func;
-  MSVCRT_atexit_registered++;
-  UNLOCK_EXIT;
+  MSVCRT__register_onexit_function(&MSVCRT_atexit_table, func);
+
   return func;
 }
 
@@ -359,6 +328,16 @@ int CDECL MSVCRT__crt_atexit(void (*func)(void))
   return MSVCRT__onexit((MSVCRT__onexit_t)func) == (MSVCRT__onexit_t)func ? 0 : -1;
 }
 
+/*********************************************************************
+ *      _register_thread_local_exe_atexit_callback (UCRTBASE.@)
+ */
+void CDECL _register_thread_local_exe_atexit_callback(_tls_callback_type callback)
+{
+    TRACE("(%p)\n", callback);
+    tls_atexit_callback = callback;
+}
+
+#endif /* _MSVCR_VER>=140 */
 
 /*********************************************************************
  *		_initialize_onexit_table (UCRTBASE.@)
@@ -457,17 +436,6 @@ int CDECL MSVCRT__execute_onexit_table(MSVCRT__onexit_table_t *table)
     return 0;
 }
 
-/*********************************************************************
- *		_register_thread_local_exe_atexit_callback (UCRTBASE.@)
- */
-void CDECL _register_thread_local_exe_atexit_callback(_tls_callback_type callback)
-{
-    TRACE("(%p)\n", callback);
-    tls_atexit_callback = callback;
-}
-
-#endif /* _MSVCR_VER>=140 */
-
 #if _MSVCR_VER>=71
 /*********************************************************************
  *		_set_purecall_handler (MSVCR71.@)
diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c
index 354fab1e94..ed5bb0589c 100644
--- a/dlls/ucrtbase/tests/misc.c
+++ b/dlls/ucrtbase/tests/misc.c
@@ -125,6 +125,8 @@ static int (CDECL *p_fesetround)(int);
 static void (CDECL *p___setusermatherr)(MSVCRT_matherr_func);
 static int* (CDECL *p_errno)(void);
 static char* (CDECL *p_asctime)(const struct tm *);
+static void (CDECL *p_exit)(int);
+static int (CDECL *p__crt_atexit)(void (CDECL*)(void));
 
 static void test__initialize_onexit_table(void)
 {
@@ -429,6 +431,8 @@ static BOOL init(void)
     p___setusermatherr = (void*)GetProcAddress(module, "__setusermatherr");
     p_errno = (void*)GetProcAddress(module, "_errno");
     p_asctime = (void*)GetProcAddress(module, "asctime");
+    p__crt_atexit = (void*)GetProcAddress(module, "_crt_atexit");
+    p_exit = (void*)GetProcAddress(module, "exit");
 
     return TRUE;
 }
@@ -765,6 +769,70 @@ static void test_asctime(void)
     ok(!strcmp(ret, "Thu Jan  1 00:00:00 1970\n"), "asctime returned %s\n", ret);
 }
 
+static void test_exit(const char *argv0)
+{
+    HANDLE exit_event1, exit_event2;
+    PROCESS_INFORMATION proc;
+    STARTUPINFOA startup = {0};
+    char path[MAX_PATH];
+    DWORD ret;
+
+    exit_event1 = CreateEventA(NULL, FALSE, FALSE, "exit_event1");
+    exit_event2 = CreateEventA(NULL, FALSE, FALSE, "exit_event2");
+
+    sprintf(path, "%s misc exit", argv0);
+    startup.cb = sizeof(startup);
+    CreateProcessA(NULL, path, NULL, NULL, TRUE, 0, NULL, NULL, &startup, &proc);
+    winetest_wait_child_process(proc.hProcess);
+
+    ret = WaitForSingleObject(exit_event1, 0);
+    ok(ret == WAIT_OBJECT_0, "exit_event1 was not set (%x)\n", ret);
+
+    CloseHandle(exit_event1);
+    CloseHandle(exit_event2);
+}
+
+static void CDECL at_exit_func1(void)
+{
+    HANDLE exit_event1 = CreateEventA(NULL, FALSE, FALSE, "exit_event1");
+    HANDLE exit_event2 = CreateEventA(NULL, FALSE, FALSE, "exit_event2");
+    DWORD ret;
+
+    ok(exit_event1 != NULL, "CreateEvent failed: %d\n", GetLastError());
+    ok(exit_event2 != NULL, "CreateEvent failed: %d\n", GetLastError());
+
+    ret = WaitForSingleObject(exit_event2, 0);
+    ok(ret == WAIT_OBJECT_0, "exit_event2 was not set (%x)\n", ret);
+
+    SetEvent(exit_event1);
+    CloseHandle(exit_event1);
+    CloseHandle(exit_event2);
+}
+
+static void CDECL at_exit_func2(void)
+{
+    HANDLE exit_event1 = CreateEventA(NULL, FALSE, FALSE, "exit_event1");
+    HANDLE exit_event2 = CreateEventA(NULL, FALSE, FALSE, "exit_event2");
+    DWORD ret;
+
+    ok(exit_event1 != NULL, "CreateEvent failed: %d\n", GetLastError());
+    ok(exit_event2 != NULL, "CreateEvent failed: %d\n", GetLastError());
+
+    ret = WaitForSingleObject(exit_event1, 0);
+    ok(ret == WAIT_TIMEOUT, "exit_event1 should not be set (%x)\n", ret);
+
+    SetEvent(exit_event2);
+    CloseHandle(exit_event1);
+    CloseHandle(exit_event2);
+}
+
+static void test_call_exit(void)
+{
+    ok(!p__crt_atexit(at_exit_func1), "_crt_atexit failed\n");
+    ok(!p__crt_atexit(at_exit_func2), "_crt_atexit failed\n");
+    p_exit(0);
+}
+
 START_TEST(misc)
 {
     int arg_c;
@@ -775,7 +843,10 @@ START_TEST(misc)
 
     arg_c = winetest_get_mainargs(&arg_v);
     if(arg_c == 3) {
-        test__get_narrow_winmain_command_line(NULL);
+        if(!strcmp(arg_v[2], "cmd"))
+            test__get_narrow_winmain_command_line(NULL);
+        else if(!strcmp(arg_v[2], "exit"))
+            test_call_exit();
         return;
     }
 
@@ -791,4 +862,5 @@ START_TEST(misc)
     test_isblank();
     test_math_errors();
     test_asctime();
+    test_exit(arg_v[0]);
 }
-- 
2.17.1

[Piotr Caban]

Hi Fabian,

On 06/17/18 19:48, Fabian Maurer wrote:
> -#define LOCK_EXIT   _mlock(_EXIT_LOCK1)
> -#define UNLOCK_EXIT _munlock(_EXIT_LOCK1)
> It's still needed to hold the exit lock. Older versions of msvcrt were 
> doing it (I didn't test if newer versions are still using this lock).

Thanks,
Piotr

[Fabian Maurer]

> It's still needed to hold the exit lock. Older versions of msvcrt 
> were
> doing it (I didn't test if newer versions are still using this lock).

Thanks, I'll have an updated version soon. Though I don't see a 
good way to write a test for this lock.

Regards,
Fabian Maurer