Emscripten is not supported with multithread (pthread flags is set on example but not on the library)

[Milerius]

Resulting ld bug:

> wasm-ld: error: 'atomics' feature is used by d1_lib.o, so --shared-memory 
> must be used
>
> This one means that `dl_lib.o` was compiled with -pthread already 
> (-pthread enables the 'atomics' feature), so its atomic operations have 
> been lowered to real atomic WebAssembly instructions. However, because 
> you're not passing -pthread at link time, the linker tries to produce a 
> module with an unshared memory, which would make those atomic instructions 
> invalid.
>
> wasm-ld: error: Target feature 'atomics' used in d1_lib.o is disallowed by 
> ConcurrentScheduler.cpp.o. Use --no-check-features to suppress.
>
> This line tells us that ConcurrentScheduler.cpp.o was compiled without 
> -pthread, which means its atomic operations were lowered to non-atomic 
> WebAssembly instruction because the atomics feature was not enabled. That 
> means that ConcurrentScheduler.cpp.o is not safe to link with dl_lib.o 
> because the resulting program would not be thread-safe, even if the program 
> was thread-safe at the source level. When using the LLVM backend, the 
> wasm-ld linker helpfully recognizes this error and does not let you link an 
> unsafe program. Fastcomp would just silently link your program into a 
> thread-unsafe binary.

[Milerius]

short solution: when multithread is enabled, please link Threads::Threads to the library, not the example.

Emscripten support pthread so you can use if (UNIX OR EMSCRIPTEN) instead of just if (UNIX)

[Milerius]

Since I stoled your GitHub Actions configuration sometimes ago.

I added an emscripten build on ubuntu, if you want you can add it into yours to.

https://github.com/KomodoPlatform/antara-gaming-sdk/blob/master/.github/workflows/build.yml

So you will be able to compile your example in the web

[DaanDeMeyer]

Just to make sure I understand correctly, reproc++ needs to link against Threads::Threads because the thread-safe string sink uses a mutex which needs the threading stuff to work?

I'll look into adding an emscripten build.

[Milerius]

Not exactly.

Reproc++ need to link Threads::Threads when the option is enabled in the CMakelists.txt for emscripten otherwise the linker will be crazy

[Milerius]

the emscripten linker will look in your final program which object use pthread and which doesn't since it's javascript there is no notion of shared libraries all the symbols are regrouped. Since the pthread is missing in some object it's considers it unsafe by default

[DaanDeMeyer]

So with emscripten either all object files should be linked with -pthread or none should be linked with -pthread?

If that's the case, I'll just define remove the if(UNIX) check for the REPROC_MULTITHREADED option and link both reproc and reproc++ against it if it is enabled.

[Milerius]

Exactly we can ask @kripken but I'm 99% sure

[Milerius]

So with emscripten either all object files should be linked with -pthread or none should be linked with -pthread?

If that's the case, I'll just define remove the if(UNIX) check for the REPROC_MULTITHREADED option and link both reproc and reproc++ against it if it is enabled.

I think it's the best option. And we can check everything compile with GitHub Actions

[Milerius]

If you want I can help you to have report.ci enabled in GitHub Actions too

[Milerius]

So you can have nice report like me :p

[DaanDeMeyer]

I made the necessary changes. If REPROC_MULTITHREADED is enabled, both reproc and reproc++ will link against the system's thread library.

The report would be overkill for reproc. I'd need more tests to actually get anything out of it.

Let me know if more changes are needed.

[Milerius]

I will give it a try thank's and close the issue if it's working !

[kripken]

So with emscripten either all object files should be linked with -pthread or none should be linked with -pthread?

Yes, the linker will complain if you mix files with different wasm feature sets. So all files should be built with pthreads support, or none.

[Milerius]

Thank's for the clarification !

[Milerius]

working.