Revert "Use __stdio_exit rather then fflush(0) on exit (emscripten-core#16130)" (emscripten-core#17046)

sbc100 · web-flow · commit a8db2bb00919 · 2022-05-25T08:50:56.000-07:00
This reverts commit eff9ae7. I decided to go this route rather than try to keeping `__stdio_exit` as in emscripten-core#17044.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -22,6 +22,8 @@ See docs/process.md for more on how version tagging works.
 ------
 - Fix crash, introduced in 3.1.11, which occurred when using pointer types
   (types ending in `*`) with getValue/setValue library functions. (#17028)
+- Fix possible deadlock in multi-threaded builds that use EXIT_RUNTIME=0 with
+  ASSERTIONS enabled. This was introduced in 3.1.3 as part of #16130. (#17044)
 
 3.1.11 - 05/21/2022
 -------------------
diff --git a/emcc.py b/emcc.py
@@ -2100,12 +2100,11 @@ def phase_linker_setup(options, state, newargs, user_settings):
       ]
 
     if not settings.STANDALONE_WASM and (settings.EXIT_RUNTIME or settings.ASSERTIONS):
-      # We use __stdio_exit to shut down musl's stdio subsystems and flush
-      # streams on exit.
-      # We only include it if the runtime is exitable, or when ASSERTIONS
+      # to flush streams on FS exit, we need to be able to call fflush
+      # we only include it if the runtime is exitable, or when ASSERTIONS
       # (ASSERTIONS will check that streams do not need to be flushed,
       # helping people see when they should have enabled EXIT_RUNTIME)
-      settings.EXPORT_IF_DEFINED += ['__stdio_exit']
+      settings.EXPORT_IF_DEFINED += ['fflush']
 
     if settings.SUPPORT_ERRNO:
       # so setErrNo JS library function can report errno back to C
diff --git a/src/library_fs.js b/src/library_fs.js
@@ -1489,10 +1489,9 @@ FS.staticInit();` +
     },
     quit: () => {
       FS.init.initialized = false;
-      // Call musl-internal function to close all stdio streams, so nothing is
-      // left in internal buffers.
-#if hasExportedFunction('___stdio_exit')
-      ___stdio_exit();
+      // force-flush all streams, so we get musl std streams printed out
+#if hasExportedFunction('_fflush')
+      _fflush(0);
 #endif
       // close all of our streams
       for (var i = 0; i < FS.streams.length; i++) {
diff --git a/src/library_wasi.js b/src/library_wasi.js
@@ -231,8 +231,8 @@ var WasiLibrary = {
   $flush_NO_FILESYSTEM__deps: ['$printChar', '$printCharBuffers'],
   $flush_NO_FILESYSTEM: function() {
     // flush anything remaining in the buffers during shutdown
-#if hasExportedFunction('___stdio_exit')
-    ___stdio_exit();
+#if hasExportedFunction('_fflush')
+    _fflush(0);
 #endif
     if (printCharBuffers[1].length) printChar(1, {{{ charCode("\n") }}});
     if (printCharBuffers[2].length) printChar(2, {{{ charCode("\n") }}});
diff --git a/src/postamble.js b/src/postamble.js
@@ -371,11 +371,10 @@ function checkUnflushedContent() {
     has = true;
   }
   try { // it doesn't matter if it fails
-#if SYSCALLS_REQUIRE_FILESYSTEM == 0
-    var flush = {{{ '$flush_NO_FILESYSTEM' in addedLibraryItems ? 'flush_NO_FILESYSTEM' : 'null' }}};
-    if (flush) flush();
-#elif hasExportedFunction('___stdio_exit')
-    ___stdio_exit();
+#if SYSCALLS_REQUIRE_FILESYSTEM == 0 && '$flush_NO_FILESYSTEM' in addedLibraryItems
+    flush_NO_FILESYSTEM();
+#elif hasExportedFunction('_fflush')
+    _fflush(0);
 #endif
 #if '$FS' in addedLibraryItems && '$TTY' in addedLibraryItems
     // also flush in the JS FS layer
diff --git a/src/postamble_minimal.js b/src/postamble_minimal.js
@@ -32,12 +32,12 @@ function run() {
 
 #endif
 
-#if IN_TEST_HARNESS && hasExportedFunction('___stdio_exit')
+#if IN_TEST_HARNESS && hasExportedFunction('_flush')
   // flush any stdio streams for test harness, since there are existing
   // tests that depend on this behavior.
   // For production use, instead print full lines to avoid this kind of lazy
   // behavior.
-  ___stdio_exit();
+  _fflush();
 #endif
 
 #if EXIT_RUNTIME
diff --git a/src/runtime_wasm64.js b/src/runtime_wasm64.js
@@ -28,7 +28,7 @@ function instrumentWasmExportsForMemory64(exports) {
           var r = Number(original(BigInt(x ? x : 0)));
           return r;
         };
-      } else if (['setThrew', 'free', 'stackRestore', '__cxa_is_pointer_type'].includes(name)) {
+      } else if (['fflush', 'setThrew', 'free', 'stackRestore', '__cxa_is_pointer_type'].includes(name)) {
         // get one i64
         replacement = (x) => {
           original(BigInt(x));
diff --git a/tests/other/metadce/test_metadce_hello_O0.exports b/tests/other/metadce/test_metadce_hello_O0.exports
@@ -1,12 +1,12 @@
 __errno_location
 __indirect_function_table
-__stdio_exit
 __wasm_call_ctors
 dynCall_jiji
 emscripten_stack_get_base
 emscripten_stack_get_end
 emscripten_stack_get_free
 emscripten_stack_init
+fflush
 main
 memory
 stackAlloc
diff --git a/tests/other/metadce/test_metadce_hello_O0.funcs b/tests/other/metadce/test_metadce_hello_O0.funcs
@@ -10,22 +10,23 @@ $__lockfile
 $__lshrti3
 $__memcpy
 $__ofl_lock
+$__ofl_unlock
 $__original_main
-$__stdio_exit
 $__stdio_write
 $__syscall_getpid
 $__towrite
 $__trunctfdf2
+$__unlock
 $__unlockfile
 $__vfprintf_internal
 $__wasi_syscall_ret
 $__wasm_call_ctors
-$close_file
 $dynCall_jiji
 $emscripten_stack_get_base
 $emscripten_stack_get_end
 $emscripten_stack_get_free
 $emscripten_stack_init
+$fflush
 $fmt_fp
 $fmt_o
 $fmt_u
diff --git a/tests/other/metadce/test_metadce_hello_O0.jssize b/tests/other/metadce/test_metadce_hello_O0.jssize
@@ -1 +1 @@
-27043
+26996
diff --git a/tests/other/metadce/test_metadce_hello_O0.size b/tests/other/metadce/test_metadce_hello_O0.size
@@ -1 +1 @@
-11949
+12105
diff --git a/tests/other/metadce/test_metadce_minimal_O0.exports b/tests/other/metadce/test_metadce_minimal_O0.exports
@@ -1,12 +1,12 @@
 __errno_location
 __indirect_function_table
-__stdio_exit
 __wasm_call_ctors
 add
 emscripten_stack_get_base
 emscripten_stack_get_end
 emscripten_stack_get_free
 emscripten_stack_init
+fflush
 global_val
 memory
 stackAlloc
diff --git a/tests/other/metadce/test_metadce_minimal_O0.funcs b/tests/other/metadce/test_metadce_minimal_O0.funcs
@@ -2,14 +2,16 @@ $__errno_location
 $__lock
 $__lockfile
 $__ofl_lock
-$__stdio_exit
+$__ofl_unlock
+$__unlock
+$__unlockfile
 $__wasm_call_ctors
 $add
-$close_file
 $emscripten_stack_get_base
 $emscripten_stack_get_end
 $emscripten_stack_get_free
 $emscripten_stack_init
+$fflush
 $stackAlloc
 $stackRestore
 $stackSave
diff --git a/tests/other/metadce/test_metadce_minimal_O0.jssize b/tests/other/metadce/test_metadce_minimal_O0.jssize
@@ -1 +1 @@
-21698
+21686
diff --git a/tests/other/metadce/test_metadce_minimal_O0.size b/tests/other/metadce/test_metadce_minimal_O0.size
@@ -1 +1 @@
-686
+846
diff --git a/tests/pthread/test_pthread_stdout_after_main.c b/tests/pthread/test_pthread_stdout_after_main.c
@@ -0,0 +1,23 @@
+#include <assert.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <emscripten.h>
+
+void* thread_main(void* arg) {
+  for (int i = 0; i < 10; i++) {
+    printf("looping...\n");
+    usleep(100*1000);
+  }
+  emscripten_force_exit(0);
+  return NULL;
+}
+
+int main() {
+  pthread_t t;
+  int rc = pthread_create(&t, NULL, thread_main, NULL);
+  assert(rc == 0);
+  // Program is built without EXIT_RUNTIME so returning here
+  // does not stop the program from running.
+  return 0;
+}
diff --git a/tests/test_core.py b/tests/test_core.py
@@ -2803,6 +2803,13 @@ def test_pthread_emmalloc(self):
     self.set_setting('MALLOC', 'emmalloc')
     self.do_core_test('test_emmalloc.c')
 
+  @node_pthreads
+  def test_pthread_stdout_after_main(self):
+    # Verify that secondary threads can continue to write to stdout even
+    # after the main thread returns.  We had a regression where stdio
+    # streams were locked when the main thread returned.
+    self.do_runf(test_file('pthread/test_pthread_stdout_after_main.c'))
+
   def test_tcgetattr(self):
     self.do_runf(test_file('termios/test_tcgetattr.c'), 'success')
 
