If multiple dynamic imports have the same dependency then local resolution of names fails

[joemarshall]

I have a python extension build where:

1. There is a C++ library, which is a .so file.
2. The C++ library is then included by a bunch of cython extensions, which are also .so files.
3. On import, each of these extensions is dynamically loaded, as is the C++ library. The c++ library is loaded as a dependency using loadDynamicLibrary(LIB,{global:false},localscope)
4. The first time an extension is loaded, this works fine. The symbols are merged into the local scope in moduleLoaded. For the second extension call though, it doesn't work, because moduleLoaded gets skipped, so symbols never make their way into the local scope.

I can load the C++ library globally, but I don't like polluting the global namespace like that. Or link it into each extension statically, but that is quite a code size jump.

I think what is needed is code after this code:

if (flags.global && !dso.global) {
        dso.global = true;
        if (dso.exports !== 'loading') {
          // ^^^ if module is 'loading' - symbols merging will be eventually done by the loader.
          mergeLibSymbols(dso.exports, libName)
        }
      }

to merge the symbols into the local scope. Possibly as simple as just calling

if (dso.exports !== 'loading' && localScope) {
  Object.assign(localScope, exports);
}

If module is still 'loading' however I don't quite know what to do here? If we are async, then I guess we could wait for it to load, but if we are synchronous (and the other load is async), I guess we just have to throw an exception.

( all referring to library_dylink.js https://github.com/emscripten-core/emscripten/blob/main/src/library_dylink.js )

[sbc100]

I agree that fix looks good. However when trying to write a test for this I noticed that I can't reproduce it using dlopen because there is an early return in the native version of dlopen if you try to open the same library twice.

Is there some reason you are calling loadDynamicLibrary directly rather than using the native dlopen API?

[joemarshall]

I think the dlopen still won't work - if you have the following:

Lib1 requires LibX
Lib2 requires LibX

However you open it, when you import lib1 as local, it will load libX (in loadDynamicLibrary) also as local.

If you then dlopen lib2 as local, then loadDynamicLibrary will see the libX dependency is already loaded, and skip the missing line which does importing into the local namespace, and name resolution of symbols in libX will fail?

[sbc100]

I will try to write a dlopen regression test based on the scenario you describe.

In the mean time, can you try converting your codebase to avoid directly calling the JS API?

[joemarshall]

It's in here: https://github.com/pyodide/pyodide/blob/main/src/js/dynload.ts

It's quite a big project - I can see that there is a workaround there for another case, which just loads all sub-dependency libraries as global, but it would be nicer if it worked like it should.

It would be quite a refactor to change it to the C bit of pyodide though I think, and it isn't my project so I'm loath to mess with it.