[Idle-dev] Fix for long delays

[Douglas S. Blank]

On Mon, May 26, 2008 5:42 am, Tal Einat said:
> David Scherer wrote:
>> Here is a patch to the IDLE shipped with Python 2.5.2 that I regard as a
>> bug
>> fix.  Specifically, it makes IDLE terminate its subprocess on Windows
>> with
>> TerminateProcess (instead of just by closing a socket), and on UNIX with
>> SIGKILL instead of SIGTERM.
>
> Is this really wanted behavior?

We have added very similar code to a startup wrapper around IDLE that does
the same thing. We have the same situation where the serial port code
talking to a robot in the background thread won't let go, and we have to
kill it.

However, this fix doesn't allow you to have two IDLE's running. The second
one started will kill the first. In fact, ours kills all running Python
processes---not generally a good idea.

> Isn't killing processes with SIGKILL
> considered bad practice, to be used only when you *really* *must* have
> the process terminate *now*? (I honestly don't know this, if someone
> knows - please answer.) Similarly, are we sure that TerminateProcess
> on Windows doesn't have any side-effects?

This is especially useful on Windows, and has no other side effects (other
than killing all running Pythons).

>> The Windows change is especially important,
>> because without it IDLE rather often fails to end my programs in a
>> timely
>> fashion when I close the output window, restart the shell, or just press
>> F5
>> to run the program again.  I think, however, that the behavior on all
>> platforms should be to terminate the user program immediately whether or
>> not
>> it cooperates; that's what I expect when I give such a command to an
>> IDE,
>> and a user program running disconnected from its output terminal is not
>> a
>> good thing.
>>
> I'm not sure I understand what you mean - do you mean that you expect
> IDLE to shut down its sub-process immediately when you close it? Why
> does having the sub-process around for a few more seconds bother you?
> One could argue that it should be killed harshly if it does shut down
> cleanly in a minute or so, but that's not what you're proposing - if I
> understand correctly, you want it killed immediately no matter what.

Having the background stop quickly would be useful.

>> I'm working on Windows at the moment, so I'd greatly appreciate it if
>> someone on the list can test my changes on a platform that has os.kill.
>>
> I'd be glad to test this on an OSX 10.5 box, once we agree on the
> conceptual issues...
>
>> I suspect that the code in run.py has some quirks that exacerbate the
>> bad
>> behavior without the patch (for example, why the 10 second timeout in
>> MyHandler.exithook?), but my patch appears to make them irrelevant, as
>> well
>> as deal with really nasty ways that the running program could freeze
>> itself
>> up (e.g. a C extension that blocks without releasing the interpreter
>> lock).
>> If there are any platforms other than Windows without os.kill, it might
>> be
>> worth at least getting rid of the timeout.
>>
> IIRC the timeout is there for a reason, although it doesn't come to
> mind ATM. Perhaps Kurt or one of the other more senior IDLE developers
>
>> Comments or questions?

I don't think I would add this patch to IDLE... too many side effects. I
think I would fix it in other ways:

1) when closing idle, have it kill the one background process immediately.

2) when starting IDLE, check for a running version, and have it open the
file/use the current Python Shell. That would prevent starting multiple
shells which are trying to do the same thing (ie, connect on to serial
ports, etc).

3) allow idle to have multiple shells (in case you really do want to do that)

My $.02

-Doug

>
> You also have a bug - the second place where you check for an
> ImportError you're not setting the correct variable (SIGTERM instead
> of SIGKILL).
>
> - Tal
> _______________________________________________
> IDLE-dev mailing list
> IDLE-dev at python.org
> http://mail.python.org/mailman/listinfo/idle-dev
>


-- 
Douglas S. Blank
Associate Professor, Bryn Mawr College
http://cs.brynmawr.edu/~dblank/
Office: 610 526 6501