quickjs-ng / quickjs

QuickJS, the Next Generation: a mighty JavaScript engine
https://quickjs-ng.github.io/quickjs/
MIT License
967 stars 79 forks source link

Support Worker API on mingw if winpthread is present #440

Closed andrjohns closed 3 months ago

andrjohns commented 3 months ago

This PR enables the Worker API on Windows if the winpthreads library is available in mingw (usually installed by default as a dependency of gcc/clang), which is already checked as part of cmake's find_package(Threads) call.

The only part that I'm unsure about is that mingw does not have a native pipe() function - so I've wrapped the Windows _pipe() for compatibility

saghul commented 3 months ago

Not a fan of winpthreads. I'd rather add native threads compatibility, like we did for other locking primitives to unlock cross platform Atomics.

That would also work on Windows with MSVC for example.

andrjohns commented 3 months ago

Not a fan of winpthreads. I

Agreed, but this probably shouldn't be framed as being about winpthreads specifically - more that the behaviour should be enabled based on feature detection instead of OS

How about I change this to cmake setting HAS_PTHREAD if POSIX threads available, and then the Worker API is enabled conditional on that?

saghul commented 3 months ago

It's not pthread specific, I just don't want to have 2 separate implementations on Windows.

andrjohns commented 3 months ago

Fair enough

saghul commented 3 months ago

Thanks for understanding 🙏

If you want to take a look, my plan is to port the thread implementation from libuv, like the locks and such.

Then port the worker API to use that new jsthread* family of APIs.

Having the 262 runner ported is a separate goal IMHO, since it uses more Unix APIs we'd need to add.