Fix ocamltest process termination on Windows

[dra27]

This partly addresses the errors detected by AppVeyor in #1730.

[shindere]

Wow, thanks a lot, @dra27 ! May I ask how you discovered the problem? I am also wondering how things actually worked until now, to be honest. I'm gonna formally approve the PR right now, please feel free to merge whenever you wish.

[dra27]

Oops - it would appear when I reformatted the commit for line-lengths, I lost a comma. Hopefully that version will pass AppVeyor this time!

@shindere - this issue only comes up if the test in question spawns another process and terminates (for example, by using the exec functions). On Windows, anyone waiting on the original process handle is released at the exec call, because the code doesn't actually replace the executable image. I imagine that either any tests which did this made their output quickly enough that it didn't matter or, more likely, that there were just no tests which were doing this. When the commands are invoked by make, I expect that because it's invoked via a shell, it's doing the same thing somewhere under the hood.

In the end, I found it by getting ocamltest to tell me the actual lines it was reading from the files (the output file didn't have enough lines), then I got ocamltest to do Sys.command ("type \"" ^ file ^ "\"") |> ignore; but that command showed all the lines and finally I guessed it might be a synchronisation/flushing problem and so inserted a 1 second delay before the file comparison and suddenly it worked. Then looking at exec_tests.ml, I realised that the lines were perfectly to do with the "original" process. A fun bit of detective work - I knew about Windows job objects, but they're usually for process management ... this is a slightly different use of them!

[shindere]

David Allsopp (2018/04/26 13:47 +0000):

@shindere - this issue only comes up if the test in question spawns another process and terminates (for example, by using the exec functions).

You mean terminates without waiting itself for its child, right? Do we have tests that do that, i.e. terminate without waiting themselves for their children?

On Windows, anyone waiting on the original process handle is released at the exec call, because the code doesn't actually replace the executable image. I imagine that either any tests which did this made their output quickly enough that it didn't matter or, more likely, that there were just no tests which were doing this. When the commands are invoked by `make`, I expect that because it's invoked via a shell, it's doing the same thing somewhere under the hood.

I see, thanks.

In the end, I found it by getting ocamltest to tell me the actual lines it was reading from the files (the output file didn't have enough lines), then I got ocamltest to do `Sys.command ("type \"" ^ file ^ "\"") |> ignore;` but that command showed all the lines and finally I guessed it might be a synchronisation/flushing problem and so inserted a 1 second delay before the file comparison and suddenly it worked. Then looking at exec_tests.ml, I realised that the lines were perfectly to do with the "original" process. A fun bit of detective work - I knew about Windows job objects, but they're usually for process management ... this is a slightly different use of them!

Really cool job, many thanks. Well in a way ocamltest has to do job control, too.

[dra27]

@shindere - I'm not 100% on the detail of it, but I think that yes the issue is where the process terminates without waiting for children to terminate - it's just that, unlike on Unix, that includes the result of an exec* call.

I meant to say with the Windows Job objects that they're usually for multi-process management! They allow killing processes as groups and resource limiting between multiple processes and all kinds of other wonderful tricks not really related to the simple question of waiting for one process and everything it's spawned to terminate!

[bschommer]

This leads to a problem if the test process itself is running in a process group, since they do not nest.

[dra27]

@bschommer - sorry for the very slow response. Have you actually hit a situation where this was a problem, or is it just theoretical? I haven't attempted to check whether it does actually work, but on Windows 8/Server 2012+ jobs can nest, so it should be an historical problem, regardless.

[bschommer]

@bschommer - sorry for the very slow response. Have you actually hit a situation where this was a problem, or is it just theoretical? I haven't attempted to check whether it does actually work, but on Windows 8/Server 2012+ jobs can nest, so it should be an historical problem, regardless.

No problem, we hit this issue in our CI infrastructure which has been updated recently, I will give it a try.

[bschommer]

I tried it, the execution of the tests now works, however the test have become terribly slow. I had to kill the job after 4 hours and the tests had only reached tool-ocaml.

[dra27]

Are you doing this with MSVC - I have seen this before as well, but I haven't come up with a satisfactory test. The issue is that cl tries to call home to Microsoft each time it is run and this hangs for a while afterwards but in the process group. In the same directory as cl.exe there should be a file vctip.exe - what happens if you temporarily delete/remove this file?

[bschommer]

Yes we are using MSVC, I will give it a try without vctip.

[bschommer]

Removing vctip.exe works and the compile times are down again. Thanks for the hint.

[xavierleroy]

The issue is that cl tries to call home to Microsoft each time it is run

Am I the only one to find it distressing that systems software routinely calls home Seattle or Cupertino? (See also #9705 (comment) )

[XVilka]

This can be fixed by using clang-cl instead. It's open source, faster than MSVC, no calls home, and builds complex software like Google Chrome and Mozilla Firefox. Both are built with clang-cl by default now:

• http://blog.llvm.org/2018/03/clang-is-now-used-to-build-chrome-for.html
• https://blog.mozilla.org/nfroyd/2019/04/25/an-unexpected-benefit-of-standardizing-on-clang-cl/

They work on improving LLD compatibility with cl.exe as well: https://lld.llvm.org/windows_support.html

[dra27]

@bschommer - thanks for confirming. In which case, I shall have a look at formalising the code I wrote to investigate this into an actual fix (at the moment, ocamltest is waiting for the number of active processes in the group to drop to zero, but it's also possible to examine what the processes are, which is how I narrowed it down, so we may just need an exception for vctip.exe).

@xavierleroy - no you're definitely not alone - I was indescribably cross at the half-day lost investigating this when I'd finally had enough of a VM where inexplicably the testsuite couldn't be run! As far as I can tell, there's no way of turning the damned thing off, even if you told Visual Studio to opt out of consumer metrics (which isn't even legal in the UK+EU).

[dra27]

@XVilka - testing MSVC with software that isn't in fact MSVC doesn't solve the problem, it just increases the test matrix.

[XVilka]

@dra27 I am suggesting just drop the MSVC support and focus on clang-cl instead, as Chrome and Firefox did. So it will not increase the testing matrix.

[bschommer]

@dra27 I am suggesting just drop the MSVC support and focus on clang-cl instead, as Chrome and Firefox did. So it will not increase the testing matrix.

I'm not sure if this is an option, in contrast to Chrome and Firefox OCaml is used to compile and link code and the last time I remembered trying clang instead of MSVC I encountered some problems with some libraries (although that was some time ago and not OCaml specific).

[dra27]

Indeed, that sounds both like using a sledgehammer to crack a nut and also jumping out of the frying pan and straight into the fire (I'm feeling very metaphorous this morning!)