Fix caml pooled mode and recursive startup

Changes

@@ -85,6 +85,12 @@ Working version
   experimental.
   (Nicolás Ojeda Bär, review by David Allsopp)
 
+- #10304: Fix a crash when combining the instrumented runtime and the
+   pooled mode, and fixes some leaks in the normal runtime in pooled mode
+   or when calling caml_startup* several times.
+  (Guillaume Munch-Maccagnoni, review by Max Mouratov and Gabriel
+   Scherer)
+
 ### Code generation and optimizations:
 
 - #9876: do not cache the young_limit GC variable in a processor register.

manual/src/cmds/intf-c.etex

@@ -1748,14 +1748,13 @@ In case the shared library produced with "-output-obj" is to be loaded and
 unloaded repeatedly by a single process, care must be taken to unload the
 OCaml runtime explicitly, in order to avoid various system resource leaks.
 
-Since 4.05, "caml_shutdown" function can be used to shut the runtime down
+Since 4.06, "caml_shutdown" function can be used to shut the runtime down
 gracefully, which equals the following:
 \begin{itemize}
 \item Running the functions that were registered with "Stdlib.at_exit".
 \item Triggering finalization of allocated custom blocks (see
-section~\ref{s:c-custom}). For example, "Stdlib.in_channel" and
-"Stdlib.out_channel" are represented by custom blocks that enclose file
-descriptors, which are to be released.
+section~\ref{s:c-custom}). For example, arrays created with
+"Bigarray.create" get their memory released.
 \item Unloading the dependent shared libraries that were loaded by the runtime,
 including "dynlink" plugins.
 \item Freeing the memory blocks that were allocated by the runtime with
@@ -1764,7 +1763,9 @@ from "memory.h" for managing static (that is, non-moving) blocks of heap
 memory, as all the blocks allocated with these functions are automatically
 freed by "caml_shutdown". For ensuring compatibility with legacy C stubs that
 have used "caml_stat_*" incorrectly, this behaviour is only enabled if the
-runtime is started with a specialized "caml_startup_pooled" function.
+runtime is started with a specialized "caml_startup_pooled" function, or with
+runtime parameter "c" (``cleanup on exit'', which in addition calls
+"caml_shutdown" on exit).
 \end{itemize}
 
 As a shared library may have several clients simultaneously, it is made for

runtime/caml/major_gc.h

@@ -89,8 +89,8 @@ void major_collection (void);
 void caml_finish_major_cycle (void);
 void caml_set_major_window (int);
 
-/* Forces finalisation of all heap-allocated values,
-   disregarding both local and global roots.
+/* Forces finalisation of all custom blocks, disregarding both local
+   and global roots.
 
    Warning: finalisation is performed by means of forced sweeping, which may
    result in pointers referencing nonexistent values; therefore the function

runtime/caml/startup_aux.h

@@ -23,28 +23,15 @@
 extern void caml_init_locale(void);
 extern void caml_free_locale(void);
 
-extern void caml_init_atom_table (void);
-
-extern uintnat caml_init_percent_free;
-extern uintnat caml_init_max_percent_free;
-extern uintnat caml_init_minor_heap_wsz;
-extern uintnat caml_init_heap_chunk_sz;
-extern uintnat caml_init_heap_wsz;
 extern uintnat caml_init_max_stack_wsz;
-extern uintnat caml_init_major_window;
-extern uintnat caml_init_custom_major_ratio;
-extern uintnat caml_init_custom_minor_ratio;
-extern uintnat caml_init_custom_minor_max_bsz;
-extern uintnat caml_init_policy;
 extern uintnat caml_trace_level;
 extern int caml_cleanup_on_exit;
 
-extern void caml_parse_ocamlrunparam (void);
-
-/* Common entry point to caml_startup.
+/* Common entry point to the various caml_startup functions.
    Returns 0 if the runtime is already initialized.
-   If [pooling] is 0, [caml_stat_*] functions will not be backed by a pool. */
-extern int caml_startup_aux (int pooling);
+   If [pooling] is 1, [caml_stat_*] functions will be backed by a pool
+   that will be freed on caml_shutdown. */
+extern int caml_startup_common(int pooling);
 
 #endif /* CAML_INTERNALS */
 

runtime/startup_aux.c

@@ -20,12 +20,15 @@
 
 #include <stdio.h>
 #include "caml/backtrace.h"
-#include "caml/memory.h"
 #include "caml/callback.h"
-#include "caml/major_gc.h"
+#include "caml/custom.h"
 #ifndef NATIVE_CODE
 #include "caml/dynlink.h"
 #endif
+#include "caml/eventlog.h"
+#include "caml/gc_ctrl.h"
+#include "caml/major_gc.h"
+#include "caml/memory.h"
 #include "caml/osdeps.h"
 #include "caml/startup_aux.h"
 
@@ -37,7 +40,7 @@ extern void caml_win32_unregister_overflow_detection (void);
 CAMLexport header_t *caml_atom_table = NULL;
 
 /* Initialize the atom table */
-void caml_init_atom_table(void)
+static void init_atom_table(void)
 {
   caml_stat_block b;
   int i;
@@ -70,17 +73,19 @@ void caml_init_atom_table(void)
 
 /* Parse the OCAMLRUNPARAM environment variable. */
 
-uintnat caml_init_percent_free = Percent_free_def;
-uintnat caml_init_max_percent_free = Max_percent_free_def;
-uintnat caml_init_minor_heap_wsz = Minor_heap_def;
-uintnat caml_init_heap_chunk_sz = Heap_chunk_def;
-uintnat caml_init_heap_wsz = Init_heap_def;
+static uintnat init_percent_free = Percent_free_def;
+static uintnat init_max_percent_free = Max_percent_free_def;
+static uintnat init_minor_heap_wsz = Minor_heap_def;
+static uintnat init_heap_chunk_sz = Heap_chunk_def;
+static uintnat init_heap_wsz = Init_heap_def;
+static uintnat init_major_window = Major_window_def;
+static uintnat init_custom_major_ratio = Custom_major_ratio_def;
+static uintnat init_custom_minor_ratio = Custom_minor_ratio_def;
+static uintnat init_custom_minor_max_bsz = Custom_minor_max_bsz_def;
+static uintnat init_policy = Allocation_policy_def;
+static uintnat record_backtrace = 0;
+
 uintnat caml_init_max_stack_wsz = Max_stack_def;
-uintnat caml_init_major_window = Major_window_def;
-uintnat caml_init_custom_major_ratio = Custom_major_ratio_def;
-uintnat caml_init_custom_minor_ratio = Custom_minor_ratio_def;
-uintnat caml_init_custom_minor_max_bsz = Custom_minor_max_bsz_def;
-uintnat caml_init_policy = Allocation_policy_def;
 extern int caml_parser_trace;
 uintnat caml_trace_level = 0;
 int caml_cleanup_on_exit = 0;
@@ -100,7 +105,7 @@ static void scanmult (char_os *opt, uintnat *var)
   }
 }
 
-void caml_parse_ocamlrunparam(void)
+static void parse_ocamlrunparam(void)
 {
   char_os *opt = caml_secure_getenv (T("OCAMLRUNPARAM"));
   uintnat p;
@@ -110,25 +115,24 @@ void caml_parse_ocamlrunparam(void)
   if (opt != NULL){
     while (*opt != '\0'){
       switch (*opt++){
-      case 'a': scanmult (opt, &caml_init_policy); break;
-      case 'b': scanmult (opt, &p); caml_record_backtrace(Val_int (p));
-        break;
+      case 'a': scanmult (opt, &init_policy); break;
+      case 'b': scanmult (opt, &record_backtrace); break;
       case 'c': scanmult (opt, &p); caml_cleanup_on_exit = (p != 0); break;
-      case 'h': scanmult (opt, &caml_init_heap_wsz); break;
+      case 'h': scanmult (opt, &init_heap_wsz); break;
       case 'H': scanmult (opt, &caml_use_huge_pages); break;
-      case 'i': scanmult (opt, &caml_init_heap_chunk_sz); break;
+      case 'i': scanmult (opt, &init_heap_chunk_sz); break;
       case 'l': scanmult (opt, &caml_init_max_stack_wsz); break;
-      case 'M': scanmult (opt, &caml_init_custom_major_ratio); break;
-      case 'm': scanmult (opt, &caml_init_custom_minor_ratio); break;
-      case 'n': scanmult (opt, &caml_init_custom_minor_max_bsz); break;
-      case 'o': scanmult (opt, &caml_init_percent_free); break;
-      case 'O': scanmult (opt, &caml_init_max_percent_free); break;
+      case 'M': scanmult (opt, &init_custom_major_ratio); break;
+      case 'm': scanmult (opt, &init_custom_minor_ratio); break;
+      case 'n': scanmult (opt, &init_custom_minor_max_bsz); break;
+      case 'o': scanmult (opt, &init_percent_free); break;
+      case 'O': scanmult (opt, &init_max_percent_free); break;
       case 'p': scanmult (opt, &p); caml_parser_trace = (p != 0); break;
       case 'R': break; /*  see stdlib/hashtbl.mli */
-      case 's': scanmult (opt, &caml_init_minor_heap_wsz); break;
+      case 's': scanmult (opt, &init_minor_heap_wsz); break;
       case 't': scanmult (opt, &caml_trace_level); break;
       case 'v': scanmult (opt, &caml_verb_gc); break;
-      case 'w': scanmult (opt, &caml_init_major_window); break;
+      case 'w': scanmult (opt, &init_major_window); break;
       case 'W': scanmult (opt, &caml_runtime_warnings); break;
       case ',': continue;
       }
@@ -146,8 +150,7 @@ static int startup_count = 0;
 /* Has the runtime been shut down already? */
 static int shutdown_happened = 0;
 
-
-int caml_startup_aux(int pooling)
+int caml_startup_common(int pooling)
 {
   if (shutdown_happened == 1)
     caml_fatal_error("caml_startup was called after the runtime "
@@ -159,9 +162,41 @@ int caml_startup_aux(int pooling)
   if (startup_count > 1)
     return 0;
 
+  /* Determine options */
+#ifdef DEBUG
+  caml_verb_gc = 0x3F;
+#endif
+  parse_ocamlrunparam();
+  if (caml_cleanup_on_exit)
+    pooling = 1;
+
+#ifdef DEBUG
+  caml_gc_message(-1, "### OCaml runtime: debug mode ###\n");
+#endif
+
   if (pooling)
     caml_stat_create_pool();
 
+  /* caml_stat_alloc* functions are available after this point */
+
+  /* Initialize the domain */
+  caml_init_domain();
+
+  CAML_EVENTLOG_INIT();
+  caml_init_locale();
+#if defined(_MSC_VER) && __STDC_SECURE_LIB__ >= 200411L
+  caml_install_invalid_parameter_handler();
+#endif
+  caml_init_custom_operations();
+  caml_init_gc(init_minor_heap_wsz, init_heap_wsz,
+               init_heap_chunk_sz, init_percent_free,
+               init_max_percent_free, init_major_window,
+               init_custom_major_ratio, init_custom_minor_ratio,
+               init_custom_minor_max_bsz, init_policy);
+  caml_init_backtrace();
+  caml_record_backtrace(Val_int(record_backtrace));
+  init_atom_table();
+
   return 1;
 }
 

runtime/startup_byt.c

@@ -32,16 +32,13 @@
 #include "caml/alloc.h"
 #include "caml/backtrace.h"
 #include "caml/callback.h"
-#include "caml/custom.h"
 #include "caml/debugger.h"
 #include "caml/domain.h"
 #include "caml/dynlink.h"
-#include "caml/eventlog.h"
 #include "caml/exec.h"
 #include "caml/fail.h"
 #include "caml/fix_code.h"
 #include "caml/freelist.h"
-#include "caml/gc_ctrl.h"
 #include "caml/instrtrace.h"
 #include "caml/interp.h"
 #include "caml/intext.h"
@@ -462,26 +459,11 @@ CAMLexport void caml_main(char_os **argv)
   char_os * shared_lib_path, * shared_libs;
   char_os * exe_name, * proc_self_exe;
 
-  /* Initialize the domain */
-  caml_init_domain();
-
-  /* Determine options */
-#ifdef DEBUG
-  caml_verb_gc = 0x3F;
-#endif
-  caml_parse_ocamlrunparam();
-  CAML_EVENTLOG_INIT();
-#ifdef DEBUG
-  caml_gc_message (-1, "### OCaml runtime: debug mode ###\n");
-#endif
-  if (!caml_startup_aux(/* pooling */ caml_cleanup_on_exit))
+  if (!caml_startup_common(0)) {
+    /* startup was already done once */
     return;
+  }
 
-  caml_init_locale();
-#if defined(_MSC_VER) && __STDC_SECURE_LIB__ >= 200411L
-  caml_install_invalid_parameter_handler();
-#endif
-  caml_init_custom_operations();
   caml_ext_table_init(&caml_shared_libs_path, 8);
 
   /* Determine position of bytecode file */
@@ -536,14 +518,7 @@ CAMLexport void caml_main(char_os **argv)
   /* Read the table of contents (section descriptors) */
   caml_read_section_descriptors(fd, &trail);
   /* Initialize the abstract machine */
-  caml_init_gc (caml_init_minor_heap_wsz, caml_init_heap_wsz,
-                caml_init_heap_chunk_sz, caml_init_percent_free,
-                caml_init_max_percent_free, caml_init_major_window,
-                caml_init_custom_major_ratio, caml_init_custom_minor_ratio,
-                caml_init_custom_minor_max_bsz, caml_init_policy);
   caml_init_stack (caml_init_max_stack_wsz);
-  caml_init_atom_table();
-  caml_init_backtrace();
   /* Initialize the interpreter */
   caml_interprete(NULL, 0);
   /* Initialize the debugger, if needed */
@@ -607,42 +582,19 @@ CAMLexport value caml_startup_code_exn(
   char_os * cds_file;
   char_os * exe_name;
 
-  /* Initialize the domain */
-  caml_init_domain();
-  /* Determine options */
-#ifdef DEBUG
-  caml_verb_gc = 0x3F;
-#endif
-  caml_parse_ocamlrunparam();
-  CAML_EVENTLOG_INIT();
-#ifdef DEBUG
-  caml_gc_message (-1, "### OCaml runtime: debug mode ###\n");
-#endif
-  if (caml_cleanup_on_exit)
-    pooling = 1;
-  if (!caml_startup_aux(pooling))
+  if (!caml_startup_common(pooling)) {
+    /* startup was already done once */
     return Val_unit;
+  }
 
-  caml_init_locale();
-#if defined(_MSC_VER) && __STDC_SECURE_LIB__ >= 200411L
-  caml_install_invalid_parameter_handler();
-#endif
-  caml_init_custom_operations();
   cds_file = caml_secure_getenv(T("CAML_DEBUG_FILE"));
   if (cds_file != NULL) {
     caml_cds_file = caml_stat_strdup_os(cds_file);
   }
   exe_name = caml_executable_name();
   if (exe_name == NULL) exe_name = caml_search_exe_in_path(argv[0]);
   /* Initialize the abstract machine */
-  caml_init_gc (caml_init_minor_heap_wsz, caml_init_heap_wsz,
-                caml_init_heap_chunk_sz, caml_init_percent_free,
-                caml_init_max_percent_free, caml_init_major_window,
-                caml_init_custom_major_ratio, caml_init_custom_minor_ratio,
-                caml_init_custom_minor_max_bsz, caml_init_policy);
   caml_init_stack (caml_init_max_stack_wsz);
-  caml_init_atom_table();
-  caml_init_backtrace();
   /* Initialize the interpreter */
   caml_interprete(NULL, 0);
   /* Initialize the debugger, if needed */