darcs

Issue 916 threaded runtime makes darcs not respond to C-c when awaiting user input

Title threaded runtime makes darcs not respond to C-c when awaiting user input
Priority urgent Status resolved
Milestone 2.1.x Resolved in
Superseder Nosy List Serware, attila.lendvai, darcs-devel, dmitry.kurochkin, kowey, simonmar, thorkilnaur, tommy
Assigned To droundy
Topics Regression, UI

Created on 2008-06-11.18:27:19 by attila.lendvai, last changed 2010-06-15.21:47:55 by admin.

Messages
msg5007 (view) Author: attila.lendvai Date: 2008-06-11.18:27:17
hi!

as the subject sais. it's probably an autoconf issue:

checking GHC.Handle.fdToHandle' API... okay
checking for module System.Posix.Signals(installHandler, raiseSignal,
Handler(..), Signal,
                         sigINT, sigHUP, sigABRT, sigALRM, sigTERM,
sigPIPE,)... yes
checking siginfo.h usability... no
checking siginfo.h presence... no
checking for siginfo.h... no
checking for module Text.Regex( mkRegex, matchRegex, Regex )... no;
and neither in package text

hth,
msg5010 (view) Author: kowey Date: 2008-06-12.08:12:13
Hi Attila,

Is this a regression, for example, from darcs 1.0.9 (or darcs 2.0.0)? Or has it
never worked?
msg5011 (view) Author: kowey Date: 2008-06-12.08:13:32
My configure output appears to be the same, fwiw:

checking for module System.Posix.Signals(installHandler, raiseSignal,
Handler(..), Signal,
                         sigINT, sigHUP, sigABRT, sigALRM, sigTERM, sigPIPE,)... yes
checking siginfo.h usability... no
checking siginfo.h presence... no
checking for siginfo.h... no
msg5013 (view) Author: attila.lendvai Date: 2008-06-12.08:43:41
> Is this a regression, for example, from darcs 1.0.9 (or darcs 2.0.0)? Or has it
> never worked?

it's a regression.

darcs head from a few weeks ago worked fine. unfortunately i can't say
for sure that it also worked after i've upgraded ubuntu to 8.04, but i
_think_ the regression happened after a darcs pull/build in the darcs
repo, not after the ubuntu upgrade.

C-c is eventually processed, but only after something happens on the
tty. i.e. when i'm entering the patch message, if i press C-c nothing
happens, but if i press enter, then C-c is finally processed and the
patch is not recorded.

hth,
msg5016 (view) Author: kowey Date: 2008-06-12.09:39:49
Ok.  Unfortunately I do not know a good way for me to reproduce this.

If you could track this down to a particular (set of) patches, for example, by
experimenting with darcs obliterate, that would be really great.

A good first step would be to make sure that darcs 2.0.0 really does work on
your machine.
msg5017 (view) Author: attila.lendvai Date: 2008-06-12.10:39:41
> If you could track this down to a particular (set of) patches, for example, by
> experimenting with darcs obliterate, that would be really great.

Fri May  9 21:58:33 CEST 2008  David Roundy <droundy@darcs.net>
  * resolve Issue739: compile with threaded runtime by default.
  It turns out that the tempfile-removal cleanup wasn't particularly feasible
  without the threaded runtime.

if i unpull everything up to and including the above patch, C-c works.

please note, that to me it seems like C-c is only broken when reading
from the terminal. if i press C-c while starting up, or reading the
repo state, C-c works as expected.

also note that the signals are seemingly stacked up: when i finish the
input in the terminal after pressing C-c several times, i get multiple
messages that C-c was received...

hth,
msg5023 (view) Author: kowey Date: 2008-06-12.12:10:10
Thanks! 

Another question: is the x86-64 relevant, or does darcs do exactly what you
expect on a 32 bit machine?  Once we have that info, I'll bring David in.
msg5025 (view) Author: attila.lendvai Date: 2008-06-12.12:18:05
> Another question: is the x86-64 relevant, or does darcs do exactly what you
> expect on a 32 bit machine?  Once we have that info, I'll bring David in.

i don't see anything unusual in its behavior. everything works as
expected, except that when reading input from the terminal, C-c is
only processed after the input is finished.

hth,
msg5026 (view) Author: kowey Date: 2008-06-12.12:24:54
Sorry, I did not understand your reply.

I was asking if there was any difference between x86-64 and the more common
x86-32  in darcs's handling of C-c.  I just an informal test, and the answer is no.

Actually, answering my question, the answer is no.

Thanks.  I'll bring David in.
msg5028 (view) Author: droundy Date: 2008-06-12.16:38:51
I'm unable to reproduce this bug.  Perhaps it's dependent on which version of
the compiler you're using? I'm using ghc 6.6 on debian etch, on an x86-64
machine.  It seems probable that it's due to a change in the threaded runtime
made in ghc 6.8.  Simon, do you have any suggestions where to look?

David
msg5029 (view) Author: attila.lendvai Date: 2008-06-12.16:52:15
> I'm unable to reproduce this bug.  Perhaps it's dependent on which version of
> the compiler you're using? I'm using ghc 6.6 on debian etch, on an x86-64
> machine.  It seems probable that it's due to a change in the threaded runtime

$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 6.8.2
$ uname -a
Linux ed101 2.6.24-18-generic #1 SMP Wed May 28 19:28:38 UTC 2008
x86_64 GNU/Linux
$ gcc --version
gcc (GCC) 4.2.3 (Ubuntu 4.2.3-2ubuntu7)
msg5052 (view) Author: simonmar Date: 2008-06-16.11:44:13
Yes, this problem was reported recently on glasgow-haskell-users, it's caused by
a change we made in the 6.8 series to fix something else (sigh).  stdout and co
aren't set to non-blocking mode now, and the threaded RTS makes blocking calls
to do I/O from these Handles.  Unfortunately the blocking calls aren't
interruptible.

I'll look into whether there's anything we can do for 6.10, but 6.8.3 will
behave the same way.  One workaround is to do the I/O in a separate thread.
msg5056 (view) Author: kowey Date: 2008-06-16.12:07:15
I'm going to mark this as urgent because it seems like the kind of UI detail
which will confuse and potentially frustrate a lot of users.

Do you think we could have this worked around for 2.0.1, David?
msg5065 (view) Author: droundy Date: 2008-06-16.16:49:17
On Mon, Jun 16, 2008 at 12:07:20PM -0000, Eric Kow wrote:
> I'm going to mark this as urgent because it seems like the kind of
> UI detail which will confuse and potentially frustrate a lot of
> users.
> 
> Do you think we could have this worked around for 2.0.1, David?

I'm not sure.  Simon, what IO would we have to do in a thread, all our
IO to stdout and/or stderr? That sounds cumbersome and risky...

David
msg5067 (view) Author: attila.lendvai Date: 2008-06-16.16:57:01
>> Do you think we could have this worked around for 2.0.1, David?
>
> I'm not sure.  Simon, what IO would we have to do in a thread, all our
> IO to stdout and/or stderr? That sounds cumbersome and risky...

just a thought from the original reporter: why don't you just add a
note somewhere that it's broken when compiled with a certain version
of GHC and simply wait until it's resolved there? people who compile
their own darcs should be prepared for such constraints and reading
INSTALL files...

anyway, it's not *that* annoying.

just my 0.02,
msg5080 (view) Author: simonmar Date: 2008-06-17.08:07:07
> David Roundy <droundy@darcs.net> added the comment:
>
> On Mon, Jun 16, 2008 at 12:07:20PM -0000, Eric Kow wrote:
> > I'm going to mark this as urgent because it seems like the kind of
> > UI detail which will confuse and potentially frustrate a lot of
> > users.
> >
> > Do you think we could have this worked around for 2.0.1, David?
>
> I'm not sure.  Simon, what IO would we have to do in a thread, all our
> IO to stdout and/or stderr? That sounds cumbersome and risky...

Do you need -threaded?  An easy workaround is not to use -threaded for now.

Here's another workaround, if you need -threaded:

import System.Posix

main = do
    setFdOption stdInput NonBlockingRead True
    ...

That puts stdin in O_NONBLOCK mode, i.e. just like it was in 6.6.  FYI this is the bug that provoked the change in the first place:

http://hackage.haskell.org/trac/ghc/ticket/724

so beware if you happen to pipe the output of darcs into tee (unlikely, I guess).

You'll need #ifndef mingw32_HOST_OS.  Also note that Windows will suffer from the same problem (and did with earlier version of GHC too), but again only with -threaded.  The nonblocking workaround above doesn't work on Windows, of course - so right now the only complete fix I can think of is to do your IO in another thread.

Cheers,
        Simon
msg5092 (view) Author: droundy Date: 2008-06-17.17:43:37
On Tue, Jun 17, 2008 at 09:15:18AM +0100, Simon Marlow wrote:
> > On Mon, Jun 16, 2008 at 12:07:20PM -0000, Eric Kow wrote:
> > > I'm going to mark this as urgent because it seems like the kind of
> > > UI detail which will confuse and potentially frustrate a lot of
> > > users.
> > >
> > > Do you think we could have this worked around for 2.0.1, David?
> >
> > I'm not sure.  Simon, what IO would we have to do in a thread, all our
> > IO to stdout and/or stderr? That sounds cumbersome and risky...
> 
> Do you need -threaded?  An easy workaround is not to use -threaded for now.

We enabled threaded because switching to use System.Process required
it, since waiting on the process we called was blocking the threads
that consumed the output, which was no good.

So I guess we can either use the setFdOption partial fix--which alas
doesn't help on Windows--or we can revert all the changes to reduce
the use of temporary files.  I'd rather not revert this work.  Your
new createProcess API doesn't have these difficulties, but that's
little help at the moment.

> Here's another workaround, if you need -threaded:
> 
> import System.Posix
> 
> main = do
>     setFdOption stdInput NonBlockingRead True
>     ...
> 
> That puts stdin in O_NONBLOCK mode, i.e. just like it was in 6.6.
> FYI this is the bug that provoked the change in the first place:
> 
> http://hackage.haskell.org/trac/ghc/ticket/724
> 
> so beware if you happen to pipe the output of darcs into tee
> (unlikely, I guess).
> 
> You'll need #ifndef mingw32_HOST_OS.  Also note that Windows will
> suffer from the same problem (and did with earlier version of GHC
> too), but again only with -threaded.  The nonblocking workaround
> above doesn't work on Windows, of course - so right now the only
> complete fix I can think of is to do your IO in another thread.

I still don't understand what you mean by doing our IO in another
thread.  What thread does it need to be different from? How much of
our IO would we have to move into different threads, and how many
would be needed?

If we'd only need to do this for reading from standard input, that'd
actually be pretty well isolated.  Any chance you could send a patch
fixing askUser in src/Darcs/Utils.lhs to spawn a thread for its call
to getLine? And then also for getCharFancy in the same file, which
calls getChar? In fact, perhaps we could put the new code into
withoutProgress in Darcs.Progress, so that any code that requires that
progress reports be disabled (which is any code that prompts for
input) will definitely be run in a thread.  Then we'd have the code in
just one place.

Of course, someone else could also contribute this, but it sounds
tricky to me, since I don't really understand what the implications
are.  I guess the problem is that the exceptions we throw in response
to sigINT are always sent to the main thread, so we just want to avoid
reading from stdin in our main thread?

David
msg5106 (view) Author: simonmar Date: 2008-06-19.16:03:00
> If we'd only need to do this for reading from standard input, that'd
> actually be pretty well isolated.  Any chance you could send a patch
> fixing askUser in src/Darcs/Utils.lhs to spawn a thread for its call
> to getLine? And then also for getCharFancy in the same file, which
> calls getChar? In fact, perhaps we could put the new code into
> withoutProgress in Darcs.Progress, so that any code that requires that
> progress reports be disabled (which is any code that prompts for
> input) will definitely be run in a thread.  Then we'd have the code in
> just one place.

Ugh, I tried this and it didn't work.  The problem is that the thread doing getChar is holding a lock on the stdin Handle, but when the ^C exception is raised you have some exception handlers in darcs that perform operations on stdin like hSetBufferingMode, and these block.  So it takes 3 ^Cs before darcs notices.

In the meantime I fixed the problem in GHC's IO library, but only for Unix.  Windows still has the problem, and has done for quite some time (but only with -threaded, which is why it wasn't an issue for darcs before).

I need to think a bit more about how to fix this.

Cheers,
        Simon
msg5108 (view) Author: kowey Date: 2008-06-19.16:36:55
It may be worth noting that David has just pushed a patch for this:
 * resolve issue916: use separate thread for reading from stdin.

the key contents of which are

    +-- withThread is used to allow ctrl-C to work even while we're waiting
    +-- for user input.
    +withThread :: IO a -> IO a
    +withThread j = do m <- newEmptyMVar
    +                  forkIO (j >>= putMVar m)
    +                  takeMVar m

Now darcs responds on the second C-c

There is always the option, as Attila suggests, of telling people, "Please
compile this with GHC 6.6 and not 6.8.2 [although 6.10 may be fine]. Otherwise,
do not be suprised if darcs does not respond to C-c"  (Come to think of this, if
we go this route, we may want to check if it now also take two tries with a
6.6-compiled darcs?).  Anyway, said route seems mildly icky.

As to the actual seriousness of this, I upgraded this bug because I had just
visions of Hypothetical User hitting C-c a few times and growing increasingly
agitated.  Maybe I'm wrong.
msg5109 (view) Author: kowey Date: 2008-06-19.16:45:32
To clarify, it responds on the second C-c for promptCharFancy, and on the first
one for C-c.  Going to go ahead and mark this as resolved, though.

I think we can satisfice with two C-c's because if users are anything like me,
they have picked up the 'if C-c doesn't work the first time, try it again' habit
msg5110 (view) Author: droundy Date: 2008-06-19.17:25:55
Is there any reason why we couldn't also do the

setFdOption stdInput NonBlockingRead True

option before reading from stdin? We could use something akin to the
already-existing withoutNonBlock function, except the opposite:  withNonBlock. 
I imagine that would give us a solution that lets the first ctrl-C through?

David

David

David
msg5112 (view) Author: simonmar Date: 2008-06-20.10:15:10
> David Roundy <droundy@darcs.net> added the comment:
>
> Is there any reason why we couldn't also do the
>
> setFdOption stdInput NonBlockingRead True
>
> option before reading from stdin? We could use something akin to the
> already-existing withoutNonBlock function, except the opposite:
> withNonBlock.
> I imagine that would give us a solution that lets the first ctrl-C
> through?

Here's the simplest solution for Unix:

myGetChar :: IO Char
myGetChar = do threadWaitRead 0; getChar

(thanks to Judah on glasgow-haskell-users for suggesting that).

It doesn't work on Windows sadly because threadWaitRead doesn't work on Windows.  I've just spent the last two hours trying to figure out how to make it work, and I've basically come to the conclusion that it can't be done - there's no sane way to implement threadWaitRead for the console in particular.  The problem is that the Windows console tracks all kinds of events, and WaitForMultipleObjects() (the equivalent of select()) will return true if the console has any pending events, but the trouble is that these events might not be actual characters that can be read (e.g. key-up events), so when you subsequently call read() on the console it will block.  So you can try to filter out all the uninteresting events, but then you have to worry about the fact that e.g. "shift" is a key-down event but doesn't generate a character.  At this point I gave up.

So I suggest doing the withThread thing on Windows, and the threadWaitRead thing on Unix.

(also I don't understand why the withThread solution requires two ^Cs for you but three ^Cs for me when I tried it).

Cheers,
        Simon
msg5119 (view) Author: droundy Date: 2008-06-20.17:03:46
On Fri, Jun 20, 2008 at 11:14:56AM +0100, Simon Marlow wrote:
> 
> > David Roundy <droundy@darcs.net> added the comment:
> >
> > Is there any reason why we couldn't also do the
> >
> > setFdOption stdInput NonBlockingRead True
> >
> > option before reading from stdin? We could use something akin to the
> > already-existing withoutNonBlock function, except the opposite:
> > withNonBlock.
> > I imagine that would give us a solution that lets the first ctrl-C
> > through?
> 
> Here's the simplest solution for Unix:
> 
> myGetChar :: IO Char
> myGetChar = do threadWaitRead 0; getChar
> 
> (thanks to Judah on glasgow-haskell-users for suggesting that).

Sounds reasonable to me.

> It doesn't work on Windows sadly because threadWaitRead doesn't work
> on Windows.  I've just spent the last two hours trying to figure out
> how to make it work, and I've basically come to the conclusion that
> it can't be done - there's no sane way to implement threadWaitRead
> for the console in particular.  The problem is that the Windows
> console tracks all kinds of events, and WaitForMultipleObjects()
> (the equivalent of select()) will return true if the console has any
> pending events, but the trouble is that these events might not be
> actual characters that can be read (e.g. key-up events), so when you
> subsequently call read() on the console it will block.  So you can
> try to filter out all the uninteresting events, but then you have to
> worry about the fact that e.g. "shift" is a key-down event but
> doesn't generate a character.  At this point I gave up.
> 
> So I suggest doing the withThread thing on Windows, and the
> threadWaitRead thing on Unix.
> 
> (also I don't understand why the withThread solution requires two
> ^Cs for you but three ^Cs for me when I tried it).

Might it help to make the ctrl-C handler use a distinct thread when
writing to stdout/stderr? It seems like then the exiting might proceed
without multiple ctrl-Cs?
-- 
David Roundy
Department of Physics
Oregon State University
msg5137 (view) Author: simonmar Date: 2008-06-23.13:19:47
> From: David Roundy [mailto:bugs@darcs.net]
> Might it help to make the ctrl-C handler use a distinct thread when
> writing to stdout/stderr? It seems like then the exiting might proceed
> without multiple ctrl-Cs?

No, I don't think so.  The ctrl-C handler runs successfully (in another thread) and raises the exception in the main thread, but then the main thread executes various exception handlers that try to perform operations on stdin, and those operations block, requiring further ^C exceptions to unblock them again.  The operations block because there's another thread stuck in getChar holding a lock on stdin.

The only real fix for this on Windows is for us to implement the IO manager on Windows like we have on Unix.  It's a big job though.

Cheers,
        Simon
History
Date User Action Args
2008-06-11 18:27:19attila.lendvaicreate
2008-06-12 08:10:55koweylinkissue917 superseder
2008-06-12 08:12:15koweysetpriority: bug
nosy: + kowey
status: unread -> unknown
messages: + msg5010
2008-06-12 08:13:33koweysetnosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5011
2008-06-12 08:43:43attila.lendvaisetnosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5013
2008-06-12 09:39:52koweysettopic: + Regression
nosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5016
assignedto: attila.lendvai
2008-06-12 10:39:43attila.lendvaisetnosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5017
2008-06-12 12:10:12koweysetnosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5023
2008-06-12 12:18:07attila.lendvaisetnosy: tommy, beschmi, kowey, dagit, attila.lendvai
messages: + msg5025
2008-06-12 12:24:56koweysetnosy: + droundy
assignedto: attila.lendvai -> droundy
messages: + msg5026
title: C-c doesn't work in head on ubuntu 8.04 x86-64 -> threaded runtime makes darcs not respond to C-c when awaiting user input
2008-06-12 16:38:53droundysetnosy: + simonmar
messages: + msg5028
2008-06-12 16:52:17attila.lendvaisetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai
messages: + msg5029
2008-06-16 11:44:15simonmarsetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai
messages: + msg5052
2008-06-16 12:07:20koweysetpriority: bug -> urgent
nosy: + Serware
topic: + UI, Target-2.0
messages: + msg5056
2008-06-16 16:49:19droundysetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, Serware
messages: + msg5065
2008-06-16 16:57:03attila.lendvaisetnosy: + serware
messages: + msg5067
2008-06-17 08:07:10simonmarsetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, Serware
messages: + msg5080
2008-06-17 17:43:40droundysetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, Serware
messages: + msg5092
2008-06-19 16:03:06simonmarsetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, Serware
messages: + msg5106
2008-06-19 16:36:59koweysetnosy: + serware, - serware
messages: + msg5108
2008-06-19 16:45:34koweysetstatus: unknown -> resolved-in-unstable
nosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, Serware
messages: + msg5109
2008-06-19 17:25:57droundysetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, Serware
messages: + msg5110
2008-06-20 10:15:14simonmarsetnosy: + serware
messages: + msg5112
2008-06-20 17:03:50droundysetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, serware, Serware
messages: + msg5119
2008-06-23 13:19:51simonmarsetnosy: droundy, tommy, beschmi, kowey, dagit, simonmar, attila.lendvai, serware, serware, Serware
messages: + msg5137
2008-08-18 14:50:09koweysettopic: + Target-2.1, - Target-2.0
nosy: - serware
2009-04-22 03:29:57twbsetstatus: resolved-in-unstable -> resolved
nosy: + dmitry.kurochkin, simon, thorkilnaur
2009-08-06 17:59:01adminsetnosy: + markstos, jast, darcs-devel, zooko, mornfall, - droundy, simonmar, attila.lendvai, serware
2009-08-06 21:07:46adminsetnosy: - beschmi
2009-08-10 22:21:17adminsetnosy: + serware, simonmar, attila.lendvai, - markstos, darcs-devel, zooko, jast, mornfall
2009-08-11 00:16:58adminsetnosy: - dagit
2009-08-25 17:44:07adminsetnosy: + darcs-devel, - simon
2009-08-27 14:18:16adminsetnosy: tommy, kowey, darcs-devel, simonmar, attila.lendvai, thorkilnaur, dmitry.kurochkin, serware, Serware
2009-10-23 22:34:54adminsetnosy: + attila_lendvai, - attila.lendvai
2009-10-23 22:38:21adminsetnosy: + marlowsd, - simonmar
2009-10-23 22:43:23adminsetnosy: - Serware
2009-10-23 23:28:18adminsetnosy: + Serware, - serware
2009-10-23 23:36:52adminsetnosy: + simonmar, - marlowsd
2009-10-23 23:48:53adminsetnosy: + attila.lendvai, - attila_lendvai
2010-06-15 21:47:53adminsetmilestone: 2.1.x
2010-06-15 21:47:55adminsettopic: - Target-2.1