Refactored (again) lib init/fini mecanism

3 files changed, 138 insertions, 165 deletions

diff --git a/src/librarian/librarian.c b/src/librarian/librarian.c
index 6ed71edc..5504e961 100755
--- a/src/librarian/librarian.c
+++ b/src/librarian/librarian.c
@@ -29,39 +29,7 @@ lib_t *NewLibrarian(box64context_t* context, int ownlibs)
 
     return maplib;
 }
-static void freeLibraryRecurse(lib_t *maplib, x64emu_t *emu, int idx, char *freed, library_t* owner) {
-    if (freed[idx]) return; // Already freed
-    
-    library_t *lib = maplib->libraries[idx];
-    if(lib==owner) return;  // don't free owner of maplib
-    freed[idx] = 1; // Avoid infinite loops
-    printf_log(LOG_DEBUG, "Unloading %s\n", lib->name);
-    for (int i = lib->dependedby.size - 1; i >= 0; --i) {
-        int j;
-        for (j = 0; j < maplib->libsz; ++j) {
-            if (lib->dependedby.libs[i] == maplib->libraries[j]) break;
-        }
-        if (j == maplib->libsz) {
-            // dependant lib already freed
-            // library as been freed already
-            continue;
-        }
-        if (freed[j] == 1) {
-            printf_log(LOG_DEBUG, "Cyclic dependancy detected (cycle is between %s and %s)\n", lib->name, lib->dependedby.libs[i]->name);
-            continue;
-        }
-        freeLibraryRecurse(maplib, emu, j, freed, owner);
-        if (freed[idx] != 1) {
-            printf_log(LOG_DEBUG, "Note: library already freed (cyclic dependancy break)\n");
-            return;
-        }
-    }
-    
-    library_t *ptr = maplib->libraries[idx];
-    if(maplib->ownlibs/* && (!maplib->owner || ptr->maplib==maplib)*/)
-        Free1Library(&ptr, emu);
-    freed[idx] = 2;
-}
+
 void FreeLibrarian(lib_t **maplib, x64emu_t *emu)
 {
     // should that be in reverse order?
@@ -70,24 +38,14 @@ void FreeLibrarian(lib_t **maplib, x64emu_t *emu)
 
     library_t* owner = (*maplib)->owner;
     (*maplib)->owner = NULL;    // to avoid recursive free...
+
+    // free the memory only. All the uninit logic is elsewhere
+
     
     if((*maplib)->ownlibs && (*maplib)->libsz) {
-        printf_log(LOG_DEBUG, "Closing %d libs from maplib %p\n", (*maplib)->libsz, *maplib);
-        char *freed = (char*)box_calloc((*maplib)->libsz, sizeof(char));
-        if (!freed) {
-            printf_log(LOG_INFO, "Failed to malloc freed table, using old algorithm (a crash is likely)\n");
-            for (int i=(*maplib)->libsz-1; i>=0; --i) 
-                if((*maplib)->libraries[i]!=owner) {
-                    printf_log(LOG_DEBUG, "Unloading %s\n", (*maplib)->libraries[i]->name);
-                    Free1Library(&(*maplib)->libraries[i], emu);
-                }
-        } else {
-            for (int i=(*maplib)->libsz-1; i>=0; --i) {
-                freeLibraryRecurse(*maplib, emu, i, freed, owner);
-            }
-            memset((*maplib)->libraries, 0, (*maplib)->libsz*sizeof(library_t*)); // NULL = 0 anyway
-            (*maplib)->libsz = 0;
-            box_free(freed);
+        for(int i=0; i<(*maplib)->libsz; ++i) {
+            printf_log(LOG_DEBUG, "Unloading %s\n", (*maplib)->libraries[i]->name);
+            Free1Library(&(*maplib)->libraries[i], emu);
         }
     }
     box_free((*maplib)->libraries);
@@ -203,14 +161,15 @@ static void MapLibRemoveMapLib(lib_t* dest, lib_t* src)
     }
 }
 
-int AddNeededLib_add(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib, int local, const char* path, box64context_t* box64, x64emu_t* emu)
+int AddNeededLib_add(lib_t* maplib, int local, needed_libs_t* needed, int n, box64context_t* box64, x64emu_t* emu)
 {
+    const char* path = needed->names[n];
     printf_log(LOG_DEBUG, "Trying to add \"%s\" to maplib%s\n", path, local?" (local)":"");
     // first check if lib is already loaded
     library_t *lib = getLib(my_context->maplib, path);
     if(lib) {
-        add_neededlib(neededlibs, lib);
-        if (lib && deplib) add_dependedbylib(&lib->dependedby, deplib);
+        IncRefCount(lib);   // increment cntref
+        needed->libs[n] = lib;
         printf_log(LOG_DEBUG, "Already present in maplib => success\n");
         return 0;
     }
@@ -218,6 +177,8 @@ int AddNeededLib_add(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib
     lib = getLib(my_context->local_maplib, path);
     if(lib) {
         printf_log(LOG_DEBUG, "Already present in local_maplib => success\n");
+        needed->libs[n] = lib;
+        IncRefCount(lib);   // increment cntref
         if(local) {
             // add lib to maplib...
             if(maplib) {
@@ -239,20 +200,15 @@ int AddNeededLib_add(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib
                 MapLibAddLib(my_context->maplib, lib);
             MapLibRemoveMapLib(my_context->local_maplib, my_context->maplib);
         }
-        add_neededlib(neededlibs, lib);
-        if (lib && deplib) add_dependedbylib(&lib->dependedby, deplib);
         return 0;
     }
     // load a new one
-    lib = NewLibrary(path, box64);
+    needed->libs[n] = lib = NewLibrary(path, box64);
     if(!lib) {
         printf_log(LOG_DEBUG, "Faillure to create lib => fail\n");
         return 1;   //Error
     }
 
-    add_neededlib(neededlibs, lib);
-    if (lib && deplib) add_dependedbylib(&lib->dependedby, deplib);
-
     // add lib now
     if(local) {
         MapLibAddLib(my_context->local_maplib, lib);
@@ -291,8 +247,10 @@ int AddNeededLib_add(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib
     return 0;
 }
 
-int AddNeededLib_init(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib, int local, int bindnow, library_t* lib, box64context_t* box64, x64emu_t* emu)
+int AddNeededLib_init(lib_t* maplib, int local, int bindnow, library_t* lib, box64context_t* box64, x64emu_t* emu)
 {
+    if(!lib)    // no lib, error is already detected, no need to return a new one
+        return 0;
     if(!maplib)
         maplib = (local)?lib->maplib:my_context->maplib;
 
@@ -303,18 +261,28 @@ int AddNeededLib_init(lib_t* maplib, needed_libs_t* neededlibs, library_t* depli
     } else {
         // it's an emulated lib, 
         // load dependancies and launch init sequence
-        if(LoadNeededLibs(box64->elfs[mainelf], maplib, &lib->needed, lib, 0, bindnow, box64, emu)) {
+        if(LoadNeededLibs(box64->elfs[mainelf], maplib, 0, bindnow, box64, emu)) {
             printf_log(LOG_DEBUG, "Failure to Add dependant lib => fail\n");
             return 1;
         }
         // some special case, where dependancies may not be correct
         if(!strcmp(GetNameLib(lib), "libCgGL.so")) {
-            const char* libs[] = {"libGL.so.1"};
-            AddNeededLib(maplib, &lib->needed, lib, 0, 0, libs, 1, box64, emu);
+            char* names[] = {"libGL.so.1"};    // TODO: it will never be uninit...
+            library_t* libs[] = { NULL };
+            needed_libs_t tmp = {0};
+            tmp.size = tmp.cap = 1;
+            tmp.names = names;
+            tmp.libs = libs;
+            AddNeededLib(maplib, 0, 0, &tmp, box64, emu);
         }
         if(!strcmp(GetNameLib(lib), "libmss.so.6")) {
-            const char* libs[] = {"libSDL-1.2.so.0", "libdl.so.2"};
-            AddNeededLib(maplib, &lib->needed, lib, 0, 0, libs, 2, box64, emu);
+            char* names[] = {"libSDL-1.2.so.0", "libdl.so.2"}; // TODO: they will never be uninit...
+            library_t* libs[] = { NULL, NULL };
+            needed_libs_t tmp = {0};
+            tmp.size = tmp.cap = 2;
+            tmp.names = names;
+            tmp.libs = libs;
+            AddNeededLib(maplib, 0, 0, &tmp, box64, emu);
         }
 
         // finalize the lib
@@ -330,28 +298,26 @@ int AddNeededLib_init(lib_t* maplib, needed_libs_t* neededlibs, library_t* depli
 }
 
 EXPORTDYN
-int AddNeededLib(lib_t* maplib, needed_libs_t* neededlibs, library_t* deplib, int local, int bindnow, const char** paths, int npath, box64context_t* box64, x64emu_t* emu)
+int AddNeededLib(lib_t* maplib, int local, int bindnow, needed_libs_t* needed, box64context_t* box64, x64emu_t* emu)
 {
+    if(!needed) // no needed libs, no problems
+        return 0;
     box64_mapclean = 0;
-    if(!neededlibs) {
-        neededlibs = box_calloc(1, sizeof(needed_libs_t));
-    }
-    int idx = neededlibs->size;
+    int ret = 0;
     // Add libs and symbol
-    for(int i=0; i<npath; ++i) {
-        if(AddNeededLib_add(maplib, neededlibs, deplib, local, paths[i], box64, emu)) {
-            printf_log(strchr(paths[i],'/')?LOG_DEBUG:LOG_INFO, "Error loading needed lib %s\n", paths[i]);
-            return 1;
+    for(int i=0; i<needed->size; ++i) {
+        if(AddNeededLib_add(maplib, local, needed, i, box64, emu)) {
+            printf_log(strchr(needed->names[i],'/')?LOG_DEBUG:LOG_INFO, "Error loading needed lib %s\n", needed->names[i]);
+            ret = 1;
         }
     }
-    int idx_end = neededlibs->size;
     // add dependant libs and init them
-    for (int i=idx; i<idx_end; ++i)
-        if(AddNeededLib_init(maplib, neededlibs, deplib, local, bindnow, neededlibs->libs[i], box64, emu)) {
-            printf_log(LOG_INFO, "Error initializing needed lib %s\n", neededlibs->libs[i]->name);
-            if(!allow_missing_libs) return 1;
+    for (int i=0; i<needed->size; ++i)
+        if(AddNeededLib_init(maplib, local, bindnow, needed->libs[i], box64, emu)) {
+            printf_log(LOG_INFO, "Error initializing needed lib %s\n", needed->names[i]);
+            if(!allow_missing_libs) ret = 1;
         }
-    return 0;
+    return ret;
 }
 
 library_t* GetLibMapLib(lib_t* maplib, const char* name)
@@ -442,16 +408,23 @@ static int GetGlobalSymbolStartEnd_internal(lib_t *maplib, const char* name, uin
     size_t size = 0;
     // check with default version...
     const char* defver = GetDefaultVersion(my_context->globaldefver, name);
+    // search in needed libs from preloaded first, in order
+    if(my_context->preload)
+        for(int i=0; i<my_context->preload->size; ++i)
+            if(GetLibGlobalSymbolStartEnd(my_context->preload->libs[i], name, start, end, size, &weak, version, vername, isLocal(self, my_context->preload->libs[i])))
+                if(*start)
+                    return 1;
     // search non-weak symbol, from older to newer (first GLOBAL object wins)
     if(GetSymbolStartEnd(GetMapSymbols(my_context->elfs[0]), name, start, end, version, vername, (my_context->elfs[0]==self || !self)?1:0, defver))
         if(*start)
             return 1;
     // TODO: create a temporary map to search lib only 1 time, and in order of needed...
     // search in needed libs from neededlibs first, in order
-    for(int i=0; i<my_context->neededlibs.size; ++i)
-        if(GetLibGlobalSymbolStartEnd(my_context->neededlibs.libs[i], name, start, end, size, &weak, version, vername, isLocal(self, my_context->neededlibs.libs[i])))
-            if(*start)
-                return 1;
+    if(my_context->neededlibs)
+        for(int i=0; i<my_context->neededlibs->size; ++i)
+            if(GetLibGlobalSymbolStartEnd(my_context->neededlibs->libs[i], name, start, end, size, &weak, version, vername, isLocal(self, my_context->neededlibs->libs[i])))
+                if(*start)
+                    return 1;
     // search in global symbols
     for(int i=0; i<maplib->libsz; ++i) {
         if(GetLibGlobalSymbolStartEnd(maplib->libraries[i], name, start, end, size, &weak, version, vername, isLocal(self, maplib->libraries[i])))
diff --git a/src/librarian/library.c b/src/librarian/library.c
index f5561674..1c18c9dc 100755
--- a/src/librarian/library.c
+++ b/src/librarian/library.c
@@ -110,10 +110,8 @@ void WrappedLib_FinishFini(library_t* lib)
         box_free(lib->w.altprefix);
     if(lib->w.altmy)
         box_free(lib->w.altmy);
-    if(lib->w.neededlibs) {
-        for(int i=0; i<lib->w.needed; ++i)
-            box_free(lib->w.neededlibs[i]);
-        box_free(lib->w.neededlibs);
+    if(lib->w.needed) {
+        free_neededlib(lib->w.needed);
     }
     FreeBridge(&lib->w.bridge);
 }
@@ -221,8 +219,9 @@ static void initWrappedLib(library_t *lib, box64context_t* context) {
             lib->getweak = WrappedLib_GetWeak;
             lib->getlocal = WrappedLib_GetLocal;
             lib->type = LIB_WRAPPED;
+            lib->w.refcnt = 1;
             // Call librarian to load all dependant elf
-            if(AddNeededLib(context->maplib, &lib->needed, lib, 0, 0, (const char**)lib->w.neededlibs, lib->w.needed, context, thread_get_emu())) {
+            if(AddNeededLib(context->maplib, 0, 0, lib->w.needed, context, thread_get_emu())) {
                 printf_log(LOG_NONE, "Error: loading a needed libs in elf %s\n", lib->name);
                 return;
             }
@@ -362,7 +361,6 @@ library_t *NewLibrary(const char* path, box64context_t* context)
         lib->name = Path2Name(path);
     lib->nbdot = NbDot(lib->name);
     lib->type = LIB_UNNKNOW;
-    lib->refcnt = 1;
     printf_log(LOG_DEBUG, "Simplified name is \"%s\"\n", lib->name);
     if(box64_nopulse) {
         if(strstr(lib->name, "libpulse.so")==lib->name || strstr(lib->name, "libpulse-simple.so")==lib->name) {
@@ -510,6 +508,25 @@ int ReloadLibrary(library_t* lib, x64emu_t* emu)
     return 0;
 }
 
+int FiniLibrary(library_t* lib, x64emu_t* emu)
+{
+    if(!lib->active)
+        return 0;   // nothing to do
+    switch (lib->type) {
+        case LIB_WRAPPED:
+            if(!--lib->w.refcnt)
+                lib->active = 0;
+            return 0;
+        case LIB_EMULATED:
+            if(emu)
+                RunElfFini(lib->e.elf, emu);
+            if(!lib->e.elf->refcnt)
+                lib->active = 0;
+            return 0;
+    }
+    return 1;   // bad type
+}
+
 void InactiveLibrary(library_t* lib)
 {
     lib->active = 0;
@@ -519,14 +536,7 @@ void Free1Library(library_t **lib, x64emu_t* emu)
 {
     if(!(*lib)) return;
 
-    if(--(*lib)->refcnt)
-        return;
-
-    if((*lib)->type==LIB_EMULATED && emu) {
-        elfheader_t *elf_header = (*lib)->e.elf;
-        RunElfFini(elf_header, emu);
-    }
-
+    // No "Fini" logic here, only memory handling
     if((*lib)->maplib)
         FreeLibrarian(&(*lib)->maplib, emu);
 
@@ -577,8 +587,6 @@ void Free1Library(library_t **lib, x64emu_t* emu)
         if((*lib)->w.symbol2map)
             kh_destroy(symbol2map, (*lib)->w.symbol2map);
     }
-    free_neededlib(&(*lib)->needed);
-    free_neededlib(&(*lib)->dependedby);
 
     box_free(*lib);
     *lib = NULL;
@@ -642,7 +650,7 @@ int GetLibWeakSymbolStartEnd(library_t* lib, const char* name, uintptr_t* start,
 }
 int GetLibGlobalSymbolStartEnd(library_t* lib, const char* name, uintptr_t* start, uintptr_t* end, size_t size, int* weak, int version, const char* vername, int local)
 {
-    if(!name[0] || !lib->active)
+    if(!name[0] || !lib || !lib->active)
         return 0;
     khint_t k;
     // get a new symbol
@@ -915,18 +923,34 @@ int getSymbolInMaps(library_t *lib, const char* name, int noweak, uintptr_t *add
     return 0;
 }
 
-int GetNeededLibN(library_t* lib) {
-    return lib->needed.size;
+int GetNeededLibsN(library_t* lib) {
+    switch (lib->type) {
+        case LIB_WRAPPED: return lib->w.needed?lib->w.needed->size:0;
+        case LIB_EMULATED: return lib->e.elf->needed->size;
+    }
+    return 0;
 }
 library_t* GetNeededLib(library_t* lib, int idx)
 {
-    if(idx<0 || idx>=lib->needed.size)
-        return NULL;
-    return lib->needed.libs[idx];
+    switch (lib->type) {
+        case LIB_WRAPPED:
+            if(idx<0 || !lib->w.needed || idx>=lib->w.needed->size)
+                return NULL;
+            return lib->w.needed->libs[idx];
+        case LIB_EMULATED:
+            if(idx<0 || idx>=lib->e.elf->needed->size)
+                return NULL;
+            return lib->e.elf->needed->libs[idx];
+    }
+    return NULL;
 }
-needed_libs_t* GetNeededLibs(library_t* lib)
+char** GetNeededLibs(library_t* lib)
 {
-    return &lib->needed;
+    switch (lib->type) {
+        case LIB_WRAPPED: return lib->w.needed?lib->w.needed->names:NULL;
+        case LIB_EMULATED: return lib->e.elf->needed->names;
+    }
+    return NULL;
 }
 
 void* GetHandle(library_t* lib)
@@ -990,74 +1014,57 @@ void AddMainElfToLinkmap(elfheader_t* elf)
     lm->l_ld = GetDynamicSection(elf);
 }
 
-static int is_neededlib_present(needed_libs_t* needed, library_t* lib)
+needed_libs_t* new_neededlib(int n)
 {
-    if(!needed || !lib)
-        return 0;
-    if(!needed->size)
-        return 0;
-    for(int i=0; i<needed->size; ++i)
-        if(needed->libs[i] == lib)
-            return 1;
-    return 0;
-}
-
-void add_neededlib(needed_libs_t* needed, library_t* lib)
-{
-    ++lib->refcnt;
-    if(!needed)
-        return;
-    if(is_neededlib_present(needed, lib))
-        return;
-    if(needed->size == needed->cap) {
-        needed->cap += 8;
-        needed->libs = (library_t**)box_realloc(needed->libs, needed->cap*sizeof(library_t*));
-    }
-    needed->libs[needed->size++] = lib;
+    needed_libs_t* ret = (needed_libs_t*)calloc(1, sizeof(needed_libs_t));
+    ret->cap = ret->size = n;
+    ret->libs = (library_t**)calloc(n, sizeof(library_t*));
+    ret->names = (char**)calloc(n, sizeof(char*));
+    return ret;
 }
 void free_neededlib(needed_libs_t* needed)
 {
-    if(!needed)
+    if(needed)
         return;
-    needed->cap = 0;
-    needed->size = 0;
-    if(needed->libs)
-        box_free(needed->libs);
+    free(needed->libs);
+    free(needed->names);
     needed->libs = NULL;
+    needed->names = NULL;
+    needed->cap = needed->size = 0;
+    free(needed);
 }
-void add_dependedbylib(needed_libs_t* dependedby, library_t* lib)
+void add1_neededlib(needed_libs_t* needed)
 {
-    if(!dependedby)
-        return;
-    if(is_neededlib_present(dependedby, lib))
+    if(!needed)
         return;
-    if(dependedby->size == dependedby->cap) {
-        dependedby->cap += 8;
-        dependedby->libs = (library_t**)box_realloc(dependedby->libs, dependedby->cap*sizeof(library_t*));
-    }
-    dependedby->libs[dependedby->size++] = lib;
-}
-void free_dependedbylib(needed_libs_t* dependedby)
-{
-    if(!dependedby)
+    if(needed->size+1<=needed->cap)
         return;
-    dependedby->cap = 0;
-    dependedby->size = 0;
-    if(dependedby->libs)
-        box_free(dependedby->libs);
-    dependedby->libs = NULL;
+    needed->cap = needed->size+1;
+    needed->libs = (library_t**)realloc(needed->libs, needed->cap*sizeof(library_t*));
+    needed->names = (char**)realloc(needed->names, needed->cap*sizeof(char*));
+    needed->size++;
 }
 
 void setNeededLibs(library_t* lib, int n, ...)
 {
     if(lib->type!=LIB_WRAPPED && lib->type!=LIB_UNNKNOW)
         return;
-    lib->w.needed = n;
-    lib->w.neededlibs = (char**)box_calloc(n, sizeof(char*));
+    lib->w.needed = new_neededlib(n);
     va_list va;
     va_start (va, n);
     for (int i=0; i<n; ++i) {
-        lib->w.neededlibs[i] = box_strdup(va_arg(va, char*));
+        lib->w.needed->names[i] = va_arg(va, char*);
     }
     va_end (va);
 }
+
+void IncRefCount(library_t* lib)
+{
+    if(lib->type!=LIB_WRAPPED && lib->type!=LIB_UNNKNOW)
+        return;
+    if(lib->type==LIB_WRAPPED) {
+        ++lib->w.refcnt;
+    } else {
+        ++lib->e.elf->refcnt;
+    }
+}
\ No newline at end of file
diff --git a/src/librarian/library_private.h b/src/librarian/library_private.h
index 021e78b6..2b2c5eff 100755
--- a/src/librarian/library_private.h
+++ b/src/librarian/library_private.h
@@ -37,8 +37,7 @@ typedef struct wlib_s {
     void*           lib;        // dlopen result
     void*           priv;       // actual private
     char*           altprefix;  // if function names are mangled..
-    int             needed;
-    char**          neededlibs;
+    needed_libs_t*  needed;
     kh_symbolmap_t  *symbolmap;
     kh_symbolmap_t  *wsymbolmap;
     kh_symbolmap_t  *mysymbolmap;
@@ -49,6 +48,7 @@ typedef struct wlib_s {
     kh_datamap_t    *wdatamap;
     kh_datamap_t    *mydatamap;
     char            *altmy;      // to avoid duplicate symbol, like with SDL1/SDL2
+    int             refcnt;      // refcounting the lib
 } wlib_t;
 
 typedef struct elib_s {
@@ -71,18 +71,11 @@ typedef struct library_s {
         wlib_t  w;     
         elib_t  e;
     };                              // private lib data
-    needed_libs_t       needed;
-    needed_libs_t       dependedby;     // used to free library
-    int                 refcnt;         // refcounting the lib
     lib_t               *maplib;        // local maplib, for dlopen'd library with LOCAL binding (most of the dlopen)
     kh_bridgemap_t      *gbridgemap;    // global symbol bridgemap
     kh_bridgemap_t      *wbridgemap;    // weak symbol bridgemap
     kh_bridgemap_t      *lbridgemap;    // local symbol bridgemap
 } library_t;
-void add_neededlib(needed_libs_t* needed, library_t* lib);
-void free_neededlib(needed_libs_t* needed);
-void add_dependedbylib(needed_libs_t* dependedby, library_t* lib);
-void free_dependedbylib(needed_libs_t* dependedby);
 
 // type for map elements
 typedef struct map_onesymbol_s {