I'm worried about changes that make stdlib follow threads's behavior and not the other way around, as threads's behavior is much more likely to be untested and not what people have been relying on. It may be the case that it is actually the better behavior (I suspect the %bytes_length vs. %string_length change is an aftermath of #596 and #772, but it looks like reverting to string_length here is an accidental change that was not required), but each would need really careful review -- maybe you should consider having separate PRs for these changes.

@gasche In this case I actually believe that it's the other way round: the threads behavior is much better tested. I'm pretty sure that a majority of real-world applications, certainly I/O-heavy systems applications, links with the threads version.

Concerning %bytes_length vs %string_length, I'm sure that this is not in any way related to threads. One of the two implementations must be the right one. The type signature of the external declaration suggests that the threads version is the right one to use, otherwise I'd seriously question the naming convention used for the primitives.

I've just updated the initial comment describing the pull request to reflect changes wrt. close_out.

I tend to agree that the threads library is probably in fact better tested.

This may be true, but for all discrepancies pointed out above, stdlib is in fact the reference version. There is one case (bytes_length) where a stdlib change not reflected in threads was in fact a bad idea, and it's a good idea to revert it, but that does not change my comment that I was worried about changing "stdlib" to match "thread" without a very careful review.

This patch touches otherlibs/threads, which is for vmthreads. I think none of the code in this directory is used when you are using system threads. I would think that no one uses vmthreads these days so the code in otherlibs/threads is likely to be the less tested one.

(I made a mistake in my previous comment above; I was thinking that the threads library being referred to was systhreads, but it is not.)

Is there a reason to keep the vmthreads library around? Does anyone use it?

@mshinwell At the time the vmthreads library was implemented, POSIX threads were not yet that widely or well-supported by operating systems. Xavier even wrote the first Linux implementation of POSIX threads (LinuxThreads), probably just so OCaml could have systhreads 😏.

I guess the question boils down to whether there are still any platforms left that don't have POSIX-threads and are worth supporting. My guess is probably not.

@xavierleroy Would you be happy removing vmthreads or not?

Does the implementation of vmthreads make sense in the context of compiling the ocaml bytecode interpreter + runtime system to WebAssembly, for instance?

You need to ask the greater OCaml user community to see if vmthreads are still in use.

The only use I know of is in JoCaml, as a more lightweight alternative to system threads. But JoCaml isn't particularly active these days...

Personally, it wouldn't break my heart to remove vmthreads altogether. System threads are enough of a mess already :-)

I can ask on the caml-list.

At worse I suppose that this could easily be split into a separate library. The only builtin support in the compiler seems to be adding +vmthreads to the search path when -vmthreads is passed. Moreover, this option is only useful for projects that don't use ocamlfind since ocamlfind already interprets -thread/-vmthreads and setups the search path accordingly.

@alainfrisch it seems that browsers are getting support for pthreads in javascript. I guess if this happens WebAssembly will get pthreads as well. In any case, lwt/async are much more popular and provides similar features, so it doesn't seem too bad to me to let anyone interested maintain it as a separate library.

BTW, we'll we need the consurtium approval to drop support for vmthreads?

@alainfrisch Thanks for the approval, Alain, and to all the reviewers. I have added the Changelog entry and rebased + squashed this branch wrt. the current trunk.