Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate

On Tue, Dec 4, 2012 at 6:12 PM, Johannes Schindelin
[off-list ref] wrote:

Hi kusma,

On Sat, 1 Dec 2012, Erik Faye-Lund wrote:

quoted

On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
[off-list ref] wrote:

quoted

Hi,

On Tue, 13 Nov 2012, Erik Faye-Lund wrote:

quoted

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.

quoted

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