Re: [msysGit] [PATCH/RFC 06/11] run-command: add kill_async() and is_async_alive()
- From
Erik Faye-Lund <kusmabite@googlemail.com>
- Date
- Nov 27, 2009, 16:04 UTC
- Message-ID
- <40aa078e0911270804i1a828ea6we1611047d37869f7@mail.gmail.com>
- In-Reply-To
- <200911262246.13342.j6t@kdbg.org>
On Thu, Nov 26, 2009 at 10:46 PM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 17 quoted lines
> On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
>> +int kill_async(struct async *async)
>> +{
>> +#ifndef WIN32
>> + return kill(async->pid, SIGTERM);
>> +#else
>> + DWORD ret = 0;
>> + if (!TerminateThread(async->tid, 0))
>> + ret = error("killing thread failed: %lu", GetLastError());
>
> Ugh! Did you read the documentation of TerminateThread()?
>
> We need to kill processes/threads when we detect that there are too many
> connections. But TerminateThread() is such a dangerous function that we
> cannot pretend that everything is good, and we continue to accept
> connections.
>Ouch, this is nasty. Something else needs to be done.
> Unless we find a different solution, I would prefer to punt and die instead. >
Do you really think it's better to unconditionally take down the entire process with an error, instead of having a relatively small chance of stuff blowing up without any sensible error? I'm not 100% convinced - but let's hope we'll find a proper fix.
Show 6 quoted lines
>> + else if (!GetExitCodeThread(async->tid, &ret))
>> + ret = error("cannot get thread exit code: %lu", GetLastError());
>
> What should the exit code be good for? The return value of this function can
> only be -1 (failure, could not kill) or 0 (success, process killed).
>I thought wait_or_whine() returned the exit-code, so I wanted to be somewhat consistent. But even if it does (haven't checked), it's probably not be worth it - I'll remove this for the next iteration.
-- Erik "kusma" Faye-Lund