From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Tue, 04 Dec 2012 17:40:44 GMT
Subject: Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
Message-ID: <CABPQNSZFDBCZh-T-HFaK3=LmKik65JoMSr4pWkuNE2VrXoo0_g@mail.gmail.com>
In-Reply-To: <alpine.DEB.1.00.1212041809150.31987@s15462909.onlinehome-server.info>

On Tue, Dec 4, 2012 at 6:12 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Sat, 1 Dec 2012, Erik Faye-Lund wrote:
>
>> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
>> >
>> >> Set a control-handler to prevent the process from terminating, and
>> >> simulate SIGINT so it can be handled by a signal-handler as usual.
>> >
>> > One thing you might want to mention is that the fgetc() handling is not
>> > thread-safe, and intentionally so: if two threads read from the same
>> > console, we are in trouble anyway.
>>
>> I'm not entirely sure if I know what you mean. Do you suggest that two
>> threads can race for setting the console ctrl-handler?
>
> That was my idea, yes.
>
>> I don't think that's the case; "SetConsoleCtrlHandler(x, TRUE)" adds a
>> console handler to the handler-chain, and SetConsoleCtrlHandler(x,
>> FALSE) removes it. If two threads add handlers, it is my understanding
>> that one of them will be run, only to report "no, no more ctrl-handling
>> needed". Since both handlers block further ctrl-handling, I don't think
>> there's a problem.
>
> My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the
> handler prematurely iff another thread wanted to install the very same
> handler (but it was already installed by the first thread).
>

Yes, but that's not how SetConsoleCtrlHandler works; if a routine x
gets added twice, it needs to be removed twice as well to not get
called. This little C-program demonstrates that:

---8<---
#include <windows.h>
#include <stdio.h>

BOOL WINAPI HandlerRoutine1(DWORD dwCtrlType)
{
        printf("Hello from handler1!\n");
        return TRUE;
}

BOOL WINAPI HandlerRoutine2(DWORD dwCtrlType)
{
        printf("Hello from handler2!\n");
        return FALSE;
}

int main()
{
        SetConsoleCtrlHandler(HandlerRoutine1, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
        GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0);
        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
        SetConsoleCtrlHandler(HandlerRoutine1, FALSE);
}
---8<---

This program outputs:
Hello from handler2!
Hello from handler1!

So since that other thread would add the console ctrl handler before
it removed it again, this should still be thread-safe as far as I can
tell.

> But as I said: for this to happen, *two* threads need to want to access
> the console for reading. In that case we'd be in bigger trouble than
> thread unsafety... We cannot read two passwords from the same console at
> the same time.

I agree with that, but I'm saying that *even if* we didn't have this
limitation, the code shouldn't be problematic.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en