New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

patch: Interactive frozen script goes to background on SIGINT #208

Closed
pyinstaller-tickets-migration opened this Issue Oct 18, 2014 · 9 comments

Comments

Projects
None yet
5 participants
@pyinstaller-tickets-migration

pyinstaller-tickets-migration commented Oct 18, 2014

Original date: 2010/09/20

Hi,

I have frozen a python program that implements a user CLI.
Normally, it dies catch KeyboardInterrupt, but when frozen this does not works anymore as expected. Setting a SIGINT handler in my python code shows that the signal indeed gets to it, but no matter how I handle the signal at the python level, it seems that the froxen process goes to the background. I use readline, and am unsure if this is connected, but when the process is in the background, I still get garbage on the terminal (my prompt it seems), like if the std in/out were not completely disconnected.

I am using a onefile frozen binary, with pyinstaller 1.4.

When I apply the following patch to make sure the wrapper pyinstaller code does not catch signals anymore, everything works as expected. The patch is next. What do you think ? Is there some problem doing that ? Does my patch makes sense ? At least it solves the problem for me....

--- pyinstaller-1.4/source/linux/main.c 2009-09-25 19:28:28.000000000 +0200
+++ pyinstaller-1.4b/source/linux/main.c    2010-09-19 20:10:09.000000000 +0200
@@ -28,6 +28,7 @@
 #include "launch.h"
 #include "getpath.h"
 #include <sys/wait.h>
+#include <signal.h>

 #ifdef FREEZE_EXCEPTIONS
 extern unsigned char M_exceptions[];
@@ -75,6 +76,12 @@
     int rc = 0;
     int pid;
     char *workpath = NULL;
+
+    // Ignore signals that should be handled by the python script
+    signal(SIGINT, SIG_IGN);
+    signal(SIGABRT, SIG_IGN);
+    signal(SIGTERM, SIG_IGN);
+
     /* atexit(cleanUp); */
 #ifdef FREEZE_EXCEPTIONS
     PyImport_FrozenModules = _PyImport_FrozenModules;
@rasky

This comment has been minimized.

Member

rasky commented Oct 18, 2014

Original date: 2010/09/20

Sounds correct, I'll look into applying it. Thanks!

@pyinstaller-tickets-migration

This comment has been minimized.

pyinstaller-tickets-migration commented Oct 18, 2014

Original date: 2010/09/20
Original reporter: *jxm AND risingtidesystems DOT COOM *

You're welcome.

If you do apply this, you might want to document the behavior for your users. I have noticed that with the patch applied, a signal handler in the python script will correctly work, but the bundled python interpreter does not automatically map SIGINT to KeyboardInterrupt. If you have a script that catches KeyboardInterrupt, you need to add the following to manually remap it:

# A fix for frozen packages
import signal
def handleSIGINT(signum, frame):
    '''
    Raise KeyboardInterrupt when we get a SIGINT.
    This is normally done by python, but even after patching
    pyinstaller 1.4 to ignore SIGINT in the C wrapper code, we
    still have to do the translation ourselves.
    '''
    raise KeyboardInterrupt
signal.signal(signal.SIGINT, handleSIGINT)
@rasky

This comment has been minimized.

Member

rasky commented Oct 18, 2014

Original date: 2010/09/20

Uhm well, then I have misunderstood what you fix achieves. Obviously, the goal should be to obtain the same behavior with the normal and frozen interpreter, without having to manually modify the Python source code.

If you say that, currently, the Python interpreter does not catch signals, there is probably something missing in the initialization process done by the bootloader. If you have time to further investigate this issue, that would be great, otherwise I'll eventually get at it.

@pyinstaller-tickets-migration

This comment has been minimized.

pyinstaller-tickets-migration commented Oct 18, 2014

Original date: 2011/05/26
Original reporter: *tramjoe DOT merin AND gmail DOT COOM *

Hi,

The problem at hand is actually two-fold:

  1. Upon reception of a SIGINT, the wrapper C code of pyInstaller does some funky things before propagating the signal to the python script. Those weird signal handling actions result in the python process going to the background, with some file descriptors still attached (at least in my case).

  2. Independantly, the propagated SIGINT that is received by the python interpreter is NOT translated to a KeyboardInterrupt, presumably because of the context being executed while that happens. Looking at python manuals (I think in the processes modules doc, cannot remember right now), I got the impression that the python interpreter actually only does the mapping to raising KeyboardInterrupt after a SIGINT under certain conditions. For instance, if the SIGINT comes while certain locks are held, no mapping to KeyboardInterrupt occurs. So my guess is that it is going to be difficult to do better than actually letting the signal propagate to the python script and not doing anything of it in the wrapper code.

My patch does fix 1) completely.
For 2), I did the "fix" on my python code side.

So I think the patch is still worth it (it DOES fix an issue), but for the other issue, I have not spent much time investigating as the python-side fix is quick and robust enough for me to use for my project.

Regards,
Jerome

@dnephin

This comment has been minimized.

dnephin commented Feb 12, 2016

We're hitting this issue very frequently with https://github.com/docker/compose (see docker/compose#2055, docker/compose#1040). We've attempted to work around the issue by implementing the signal handler in different ways, but in the end the issue is still there.

None of the issues occur when running the program from a pip install.

I'd be happy to test out any patches you might have to address the issue.

@dnephin

This comment has been minimized.

dnephin commented Feb 12, 2016

Proposed a fix in #1822, maybe it's not right, but would appreciate any feedback about why it's not.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Mar 6, 2016

I tried looking into this, but my simple program (just time.sleep(15)) aborts correctly.

Does anybody have a small example which hits this but at least every second or third run?

@dnephin

This comment has been minimized.

dnephin commented Mar 7, 2016

After looking into this again, I realized the pyinstaller bug was not exactly what I thought it was. The actual bug we're hitting is described here: docker/compose#2251 and docker/compose#1069.

When running the pyinstaller binary a SIGINT can cause the blocking queue.get() call to raise thread.error. I had worked around that by adding a try/except thread.error: raise KeyboardInterupt(), which was causing a different problem with our error handling.

I've been able to partially work around this issue by raising a different exception on thread.error(), but we still see cases where the signal handling does not work correctly when run from the pyinstaller binary (and we can't reproduce the issue when run from a pip install).

It's possible that it's related to the queue.get() exception still, but unfortunately I don't have a great minimal case to reproduce this. I suspect you'd be able to reproduce the thread.error() issue with something like this:

import Queue
queue = Queue.Queue()
queue.get(timeout=100)

and send the process a SIGINT when it's blocked.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Aug 28, 2018

#3515 introduces a new option --bootloader-ignore-signals which can be used to activate this behavior for all signals.

@htgoebel htgoebel closed this Aug 28, 2018

@CSammy CSammy referenced this issue Oct 28, 2018

Closed

Add a docker setup for development #7870

11 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment