Dont use KeyboardInterrupt to stop a build!

[bdbaddog]

This issue was originally created at: 2008-02-11 17:21:02.
This issue was reported by: belley.
belley said at 2008-02-11 17:21:02

Hi there,

SCons would often hang after pressing Ctrl-C. I started investigating and I
realized that most of the Python libraries are not really safe with respect to
asynchronous exceptions. Although,there are enough try/finally blocks to handle
exceptions thrown synchronously by the library code, the Python libraries are
not always protected against exceptions being thrown asynchronously, such as a
KeyboardInterrupt being thrown at a completely random location.

For example, the function Queue.empty() does not protect its mutex with a
try/finally block. If the KeyboardInterrupt exception gets thrown while the
mutex is held, any further attempt to access the Queue will lead to dead-lock
(explaining why SCons hangs sometimes after pressing CTRL-C). Even the
threading.Condition condition variables are not async-exception safe. It
therefore seems a lost battle to try to stop a build by raising an exception.

Instead, I have implemented a signal handler that tells the Jobs (and its
associated Taskmaster) to stop the build. I have been careful to wait after the
.sconsign file has been written back to re-install the default SIGINT signal
handler that raises a KeyboardInterrupt exception.

(Patch submitted against changeset 2659 of branches/core.)

Benoit

belley said at 2008-02-11 17:23:13

Created an attachment (id=302)
Bug fix + regression test

stevenknight said at 2008-04-12 13:54:20

Hi Benoit--

Updating the target milestone to 0.98.1, per recent bug parties. We'd like to
get this in right away.

I need some help integrating this. Since I went ahead and integrated your later
patch that adds the needs_execute() method, this patch no longer applies cleanly
to Job.py, and it's not clear to me how to merge the change. Could you take a
look and provide an updated patch, or tell me what needs to happen?

Thanks,

gregnoel said at 2008-04-14 11:26:16

*** Issue 1879 has been marked as a duplicate of this issue. ***

stevenknight said at 2008-04-14 22:25:24

Fixed (by Benoit) in next release:

http://scons.tigris.org/servlets/ReadMsg?list=dev&msgNo=5461
http://scons.tigris.org/servlets/ReadMsg?list=commits&msgNo=1342

belley attached ctrlc.patch at 2008-02-11 17:23:13.

Bug fix + regression test

gregnoel said this issue is duplicated by #1879 at 2008-04-14 11:26:17.