Re: [PATCH 0/6] Pass t5530 on Windows

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2010, 20:12 UTC
Message-ID: <7vk4tpdx9x.fsf@alter.siamese.dyndns.org>
In-Reply-To: <cover.1267889072.git.j6t@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 5 quoted lines

> Quite frankly, I don't quite know what to do with this series. On the
> one hand, it is a clean-up, but in practice it is not relevant whether
> die() kills only the async thread or the whole process because all
> callers of async die() themselves anyway when the async procedure died.
> On the other hand, it does enable threaded async procedures on POSIX...

You wrote in [PATCH 5/6]:

    A new configuration option is introduced so that the threaded
    implementation can also be used on POSIX systems. Since this option is
    intended only as playground on POSIX, but is mandatory on Windows, the
    option is not documented.

but I am wondering how much of the real world is threads-challenged these days. Here is what you have at the end of technical/api-run-command.txt:

   There are serious restrictions on what the asynchronous function can do
   because this facility is implemented by a pipe to a forked process on
   UNIX, but by a thread in the same address space on Windows:

   . It cannot change the program's state (global variables, environment,
     etc.) in a way that the caller notices; in other words, .in and .out
     are the only communication channels to the caller.

   . It must not change the program's state that the caller of the
     facility also uses.

And calling die() from async is obviously "change the program's state that the caller of the facility also uses". We didn't uncover this as a bug because the above "serious restrictions" go both ways.

If we make threaded-async the default on any platform that is thread capable, we would increase the likelihood of catching bugs that violate the latter condition. I am sure there may be downsides for going that route, but it might be better than the current situation where two major platforms use quite different underlying semantics for the same call, and rely on the program (both caller and callee) to honor the above conditions, without much tool support [*1*].

[Footnote]

*1* I sometimes wonder if people who are interseted in static analysis can help with issues like this: "In a function started by start_async() and its callees, you are not supposed to touch these globals, nor call those functions". That would be more useful than reports we occasionally get "in this codepath this variable can be used before assigned" with many false positives, that presumably come from from the canned set of rules these static checkers may have.