Title: signal.signal may raise java.lang.IllegalArgumentException
Type: Severity: normal
Components: Versions:
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amak, fwierzbicki, pekka.klarck, pjenvey, zyasoft
Priority: high Keywords: patch

Created on 2011-04-07.13:50:49 by pekka.klarck, last changed 2015-03-02.01:14:38 by zyasoft.

File name Uploaded Description Edit Remove
signal.patch pekka.klarck, 2011-04-14.19:50:11
msg6471 (view) Author: Pekka Klärck (pekka.klarck) Date: 2011-04-07.13:50:48
I got a report from a team using a tool I develop that the tool fails to start with this error:

  IllegalArgumentException: Signal already used by VM: INT

This error occurs when the tool registers signal handlers with signal.signal. The code already catches ValueError which this method is documented to possibly raise [1] but obviously IllegalArgumentException goes through.

I looked at the source of signal module distributed with Jython 2.5.1 and noticed that it calls sun.misc.Signal.handle. I then looked at its documentation [2] which tells this in the beginning of the module doc:

"""Java code cannot register a handler for signals that are already used by the Java VM implementation. The Signal.handle function raises an IllegalArgumentException if such an attempt is made."""

It seems to me that calls to sun.misc.Signal.handle should be wrapped with try/except and possible IllegalArgumentException then reraised as a ValueError. I can provide a patch if this sounds like a good idea.

msg6472 (view) Author: Pekka Klärck (pekka.klarck) Date: 2011-04-07.14:11:10
According to this StackOverflow question the underlying IllegalArgumentException problem is JVM specific:
msg6474 (view) Author: Philip Jenvey (pjenvey) Date: 2011-04-09.19:52:18
Sounds good to me
msg6484 (view) Author: Pekka Klärck (pekka.klarck) Date: 2011-04-14.19:50:11
A patch that wraps all sun.misc.Signal methods that may raise IllegalArgumentException with try/except that reraises them as ValueErrors is attached. As a result also signal.getsignal and signal.alarm may raise ValueError which they aren't documented to do, but that's probably better than raising equally undocumented Java based IAE.

I would have liked to create unit tests for these fixes but couldn't figure any easy way to do that. Probably tests could temporarily replace sun.misch.Signal.handle with a method that always raises an IAE but I'm not sure is that worth the effort.
msg6630 (view) Author: Alan Kennedy (amak) Date: 2011-09-03.02:30:23
IMHO, we should not be shipping or supporting anything in the sun.* namespace. Jython is sufficiently powerful that users are free and empowered to make use of these non-standard facilities if they wish. 

But they should not be part of the core platform, which should rely only on java.*, javax.* or open source components in other namespaces. sun.misc has no place in jython.

For example, what happens if the code is run on a IBM JVM? Should we support sun.misc.* there?

If users wish to use JVM-specific facilities, they should be prepared to deal with the consequences.

My €0,02.
msg9048 (view) Author: Jim Baker (zyasoft) Date: 2014-09-26.06:02:29
signal is the one case we use sun.misc services directlty in Jython, but they are also used in JFFI and Guava if available. However, this requires importing signal, so it's not an issue on unsupported platforms.
msg9546 (view) Author: Jim Baker (zyasoft) Date: 2015-02-22.17:41:24
Fixed as of
msg9547 (view) Author: Pekka Klärck (pekka.klarck) Date: 2015-02-22.18:33:27
Thanks for merging the patch Jim! If I would have implemented it now, I probably would have used a context manager like this instead:

from contextlib import contextmanager

def map_java_exception(exception_map):
    except tuple(exception_map) as err:
        exp = exception_map[type(err)]
        raise exp(err.getMessage())

with map_java_exception({IllegalArgumentException: ValueError}):
    # ...
Date User Action Args
2015-03-02 01:14:38zyasoftsetstatus: pending -> closed
2015-02-22 18:33:27pekka.klarcksetmessages: + msg9547
2015-02-22 17:41:24zyasoftsetstatus: open -> pending
resolution: remind -> fixed
messages: + msg9546
2015-02-19 14:39:59zyasoftsetpriority: normal -> high
2014-09-26 06:02:29zyasoftsetpriority: low -> normal
resolution: remind
messages: + msg9048
nosy: + zyasoft
2013-02-27 18:29:36fwierzbickisetpriority: low
2012-08-10 21:06:21fwierzbickisetnosy: + fwierzbicki
2011-09-03 02:30:24amaksetnosy: + amak
messages: + msg6630
2011-04-14 19:50:12pekka.klarcksetfiles: + signal.patch
keywords: + patch
messages: + msg6484
2011-04-09 19:52:18pjenveysetnosy: + pjenvey
messages: + msg6474
2011-04-07 14:11:10pekka.klarcksetmessages: + msg6472
2011-04-07 13:50:49pekka.klarckcreate