Re: Win32 CTRL-C handling (Was: interrupt handing and the GUI (bug #37672))

[Michael Goffioul]

On Wed, Nov 20, 2013 at 12:48 PM, Michael Goffioul <address@hidden> wrote:

On Tue, Nov 19, 2013 at 8:01 AM, Michael Goffioul <address@hidden> wrote:

On Tue, Nov 19, 2013 at 1:10 AM, John W. Eaton <address@hidden> wrote:

On 11/18/2013 05:58 PM, Michael Goffioul wrote:

On Mon, Nov 18, 2013 at 10:50 AM, Michael Goffioul

<address@hidden <mailto:michael.goffioul@gmail.com>> wrote:

    One main issue I'd like to raise and discuss is the interaction with
    readline. Basically when you combine octave (using gnulib), readline
    (not using gnulib) and the fact that Win32 does not have siglongjmp,
    you start having conflicting situations.

    Typically, readline installs its own SIGINT handler, but also store
    the previous handler for later use. During SIGINT handling, it does
    some internal clean-up, restore the previous handler, than
    "raise(SIGINT)" to execute the previous handler.

    OTOH, octave uses gnulib. The gnulib's signal handling is along
    those lines: block the signal, execute the handler, unblock the
    signal. If the signal handler performs a longjmp, the signal will
    stay in "blocked" state (in gnulib, this translates into having a
    dummy handler that does nothing). Now the problem is that on the
    next run of readline, the stored handler will be the gnulib's dummy
    one, and CTRL-C will end up being ignored while being in readline
    loop. This does not happen on platforms having sigsetjmp/siglongjmp,
    or more generally on POSIX platforms.

    I've worked around that issue by installing octave_interrupt_hook
    and make sure SIGINT is handled after an interrupt. Another way,
    which I tested, is to disable signal handling in readline by setting
    rl_catch_signals to 0, but I'm not sure to what extend this is a
    problem in readline itself.

Scrap that, it didn't work. It appears that gnulib is keeping
internal/static states about signal handlers, which is not really
compatible with readline making direct calls to the "signal" function
from windows runtime DLL.

I ended up disabling signals in readline. The attached path works for me
in GUI and CLI. Although as explained earlier, CTRL-C in CLI mode
doesn't work as expected when in readline, because the key sequence is
reported in the input buffer, not as a signal.

It would nice if someone can test it in MinGW.

Your change works for me with MinGW.

The attached change is my attempt to allow sleep and system to be interrupted.  It seems to work for me for the sleep function but I'm still unable to interrupt the system function.

That's because when using the GUI, no signal is really generated at the OS-level. The signal is raised in the octave process; bu raising a signal in Win32 simply means executing its handler. So SIGINT is not seen by the child processes. I tried to use GenerateConsoleCtrlEvent, but it exited octave instead of executing the ctrl-c handler. There's something I'm missing there, this requires further investigation.

For the record, I think I've identified why GenerateConsoleCtrlEvent isn't working, though I don't have a workaround at the moment. It's due to the order of SetConsoleCtrlHandler and AllocConsole. The former is done internally by the Win32 runtime DLL, when installing a signal handler for SIGINT. The latter is done by the Win32 terminal implementation. In the current octave flow, SetConsoleCtrlHandler happens before AllocConsole. The problem is then that the CTRL-C handler that was installed gets lost, because a brand new console is created. The system then reverts back to the default CTRL-C handler, which is uncleanly exiting the octave process.

The attached patch solves the problem of interrupting system calls like "system('sleep 10')" in the GUI. By avoiding installing a SIGINT handler before AllocConsole is called, one can use GenerateConsoleCtrlEvent, allowing to propagate the CTRL-C sequence to child processes.

John, could you test it in MinGW? If you're ok with it, I'll apply the patch to the default branch.

With this patch, I think we have a decent CTRL-C handling under Win32, even when using the GUI.

Michael.

ctrl-c-gui
Description: Binary data