Keep process metadata alive until reaping

src/dmosi_proc.c

@@ -89,34 +89,6 @@ static dmosi_process_id_t generate_process_id()
     return next_process_id++;
 }
 
-/**
- * @brief Resolve the heap-allocation bucket that belongs to a process
- *
- * process->module_name is *not* unique - every concurrently-running instance of the
- * same module shares it (e.g. a shell spawning another instance of itself), so tagging
- * a process's own allocations with it would pool them into one bucket shared with every
- * other instance, attributing this process's memory to whichever instance the accounting
- * happens to land on rather than to this one. The context's AllocatorName is the actual
- * per-instance-unique identity ("<module name>#<counter>", see the doc comment on
- * Dmod_Context_t::AllocatorName) - the same one Dmod_GetCurrentAllocatorNameEx() would
- * report for this process's own code, were it the one currently executing.
- *
- * dmod has no public accessor that resolves an arbitrary (non-current) context's
- * AllocatorName - only Dmod_GetCurrentAllocatorNameEx(), which resolves whichever
- * context the *calling* thread is in, not @p process's - so this reads the field
- * directly as a deliberate, narrow exception to going through dmod's API.
- *
- * @param process Process handle (already validated by the caller)
- * @return const char* Allocator/bucket name to tag heap allocations for this process with,
- *         falling back to module_name when the process has no context linked (e.g. one
- *         created directly via dmosi_process_create(), or one whose context has already
- *         been unloaded)
- */
-static const char* process_allocator_name(dmosi_process_t process)
-{
-    return process->context != NULL ? process->context->AllocatorName : process->module_name;
-}
-
 /**
  * @brief Detach and invoke all exit callbacks registered on a process
  *
@@ -210,7 +182,7 @@ static bool kill_threads( dmosi_process_t process, int status )
         return true; // No threads to kill
     }
 
-    dmosi_thread_t* threads = Dmod_MallocEx(sizeof(dmosi_thread_t) * count, process->module_name);
+    dmosi_thread_t* threads = Dmod_MallocEx(sizeof(dmosi_thread_t) * count, DMOD_CURRENT_ALLOCATOR);
     if(!threads)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for thread handles while killing process %s of module %s\n", process->name, process->module_name);
@@ -230,7 +202,7 @@ static bool kill_threads( dmosi_process_t process, int status )
         return true;
     }
 
-    process_kill_completion_t* completion = Dmod_MallocEx(sizeof(*completion), process->module_name);
+    process_kill_completion_t* completion = Dmod_MallocEx(sizeof(*completion), DMOD_CURRENT_ALLOCATOR);
     if(!completion)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for exit-callback tracking while killing process %s of module %s\n", process->name, process->module_name);
@@ -462,7 +434,7 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, dmosi_process_t, _process_create,(const
         return NULL;
     }
     module_name = module_name ? module_name : DMOSI_SYSTEM_MODULE_NAME;
-    dmosi_process_t process = Dmod_MallocEx(sizeof(struct dmosi_process), module_name );
+    dmosi_process_t process = Dmod_MallocEx(sizeof(struct dmosi_process), DMOD_CURRENT_ALLOCATOR );
     if(!process)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for process %s of module %s\n", name, module_name);
@@ -472,7 +444,7 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, dmosi_process_t, _process_create,(const
     process->magic = MAGIC_NUMBER;
     process->exit_status = 0;
     process->state = DMOSI_PROCESS_STATE_RUNNING;
-    process->name = Dmod_StrDup(name);
+    process->name = Dmod_StrDupEx(name, DMOD_CURRENT_ALLOCATOR);
     process->pid = generate_process_id();
     process->uid = 0;
     process->pwd = NULL;
@@ -797,7 +769,7 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, int, _process_set_pwd, (dmosi_process_t
     }
 
     // Allocate and copy new pwd
-    process->pwd = Dmod_StrDup(pwd);
+    process->pwd = Dmod_StrDupEx(pwd, DMOD_CURRENT_ALLOCATOR);
     if(!process->pwd)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for working directory\n");
@@ -840,20 +812,16 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, int, _process_set_command, (dmosi_proces
     }
 
     // Allocate and copy new command
-    process->command = Dmod_StrDup(command);
+    process->command = Dmod_StrDupEx(command, DMOD_CURRENT_ALLOCATOR);
     if(!process->command)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for command\n");
         return -ENOMEM;
     }
 
-    // Dmod_StrDup() tags the allocation under whatever allocator identity is ambient at
-    // the call site - typically the process's own module, but the *spawning* module while
-    // this is being called from the module-start API (see dmod_spawn_module_internal in
-    // dmosi), since the new process's own thread has not started running yet at that point.
-    // Move it into the process's own bucket instead of leaving it permanently mistagged to
-    // whoever happened to call this - a no-op if the backend can't retag.
-    Dmod_RetagEx(process->command, process_allocator_name(process));
+    /* Process metadata outlives its module context: Dmod_Unload collects the
+     * context before a parent reaps this process. Keep the command under the
+     * process implementation's allocator, like name/pwd/stream paths. */
 
     return 0;
 }
@@ -883,10 +851,8 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, int, _process_set_command_args, (dmosi_p
         total_length += strlen(argv[i]);
     }
 
-    // Allocated directly under the process's own allocator bucket, unlike Dmod_StrDup()
-    // in _process_set_command() - so unlike that path, there is nothing to retag
-    // afterwards, and no intermediate string for a caller to build and free first.
-    char* command = Dmod_MallocEx(total_length + 1, process_allocator_name(process));
+    // Explicitly owned by the process until process_destroy(), not module GC.
+    char* command = Dmod_Malloc(total_length + 1);
     if(!command)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for command\n");
@@ -971,7 +937,7 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, int, _process_set_stream, (dmosi_process
         return -EIO;
     }
 
-    char* new_path = Dmod_StrDup(path);
+    char* new_path = Dmod_StrDupEx(path, DMOD_CURRENT_ALLOCATOR);
     if(!new_path)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for stream path\n");
@@ -1109,7 +1075,7 @@ DMOD_INPUT_API_DECLARATION( dmosi, 1.0, dmosi_process_exit_callback_handle_t, _p
         return NULL;
     }
 
-    struct dmosi_process_exit_callback* node = Dmod_MallocEx(sizeof(*node), process->module_name);
+    struct dmosi_process_exit_callback* node = Dmod_MallocEx(sizeof(*node), DMOD_CURRENT_ALLOCATOR);
     if(!node)
     {
         DMOD_LOG_ERROR("Failed to allocate memory for exit callback registration on process %s\n", process->name);

tests/CMakeLists.txt

@@ -18,3 +18,13 @@ target_link_options(${PROJECT_NAME} PRIVATE -T ${CMAKE_CURRENT_SOURCE_DIR}/main.
 
 # Register test with CTest
 add_test(NAME dmosi_proc_tests COMMAND ${PROJECT_NAME})
+
+add_executable(command_lifetime command_lifetime.c)
+dmod_set_current_allocator(command_lifetime)
+target_link_libraries(command_lifetime dmod dmosi_proc dmosi)
+target_link_options(command_lifetime PRIVATE -L ${DMOD_DIR}/scripts
+    -T ${CMAKE_CURRENT_SOURCE_DIR}/main.ld
+    -Wl,--wrap=Dmod_MallocEx,--wrap=Dmod_FreeEx,--wrap=Dmod_FreeModule,--wrap=Dmod_RetagEx)
+add_test(NAME command_lifetime COMMAND command_lifetime)
+
+target_compile_options(command_lifetime PRIVATE -UNDEBUG)

tests/command_lifetime.c

@@ -0,0 +1,67 @@
+/* Exercise module GC between command creation and process reaping using an
+ * allocator that tracks the production SAL ownership tags. */
+#include "dmod.h"
+#include "dmosi.h"
+#include <assert.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+struct allocation {void* p;char owner[128];};
+static struct allocation allocations[128];
+void* __real_Dmod_MallocEx(size_t size,const char* owner);
+void __real_Dmod_FreeEx(void* p,bool concatenate);
+void* __wrap_Dmod_MallocEx(size_t size,const char* owner)
+{
+    void* p=__real_Dmod_MallocEx(size,owner);
+    if(!p)return NULL;
+    for(unsigned i=0;i<128;++i)if(!allocations[i].p){
+        allocations[i].p=p;
+        snprintf(allocations[i].owner,sizeof(allocations[i].owner),"%s",owner?owner:"");return p;
+    }
+    abort();
+}
+static int live(const void* p)
+{for(unsigned i=0;i<128;++i)if(allocations[i].p==p)return 1;return 0;}
+void __wrap_Dmod_FreeEx(void* p,bool concatenate)
+{
+    if(!p)return;
+    assert(live(p));
+    for(unsigned i=0;i<128;++i)if(allocations[i].p==p)allocations[i].p=NULL;
+    __real_Dmod_FreeEx(p,concatenate);
+}
+bool __wrap_Dmod_RetagEx(void* p,const char* owner)
+{
+    for(unsigned i=0;i<128;++i)if(allocations[i].p==p){
+        snprintf(allocations[i].owner,sizeof(allocations[i].owner),"%s",owner?owner:"");return true;
+    }
+    return false;
+}
+void __wrap_Dmod_FreeModule(const char* owner)
+{
+    for(unsigned i=0;i<128;++i)
+        if(allocations[i].p && !strcmp(allocations[i].owner,owner))
+            __wrap_Dmod_FreeEx(allocations[i].p,false);
+}
+int main(void)
+{
+    for(int args=0;args<2;++args){
+        dmosi_process_t process=dmosi_process_create("lifetime","child",NULL);
+        assert(process);
+        Dmod_Context_t context={0};
+        strcpy(context.AllocatorName,"child#lifetime");
+        assert(dmosi_process_set_context(process,&context)==0);
+        char* argv[]={"child","argument"};
+        assert((args?dmosi_process_set_command_args(process,2,argv):
+                      dmosi_process_set_command(process,"child argument"))==0);
+        const char* command=dmosi_process_get_command(process);
+        assert(live(command));
+        Dmod_FreeModule(context.AllocatorName);
+        assert(live(command));
+        assert(!strcmp(command,"child argument"));
+        assert(dmosi_process_set_context(process,NULL)==0);
+        dmosi_process_destroy(process);
+        assert(!live(command));
+    }
+    for(unsigned i=0;i<128;++i)assert(!allocations[i].p);
+    return 0;
+}