Allow bpython/bpdb outside main thread

[maxnordlund]

Because signal.signal is only allowed to be called from the main thread, if you use bpdb in another thread it will crash upon entering the repl. By ignoring the resulting ValueError the show can go on.

This makes bpdb useful in Django's runserver, even with threading and/or live reload on.

A question is what to tell the user when this happens? Just ignore it like here, or log something? What on which level?

Feedback much appreciated.

Closes #555 I think

[thomasballinger]

Cool, thanks @maxnordlund. I think we should use this, but first we should document what stops working when these signal handlers can't be installed. Hm yeah maybe we should let the user know too? A startup message that says what won't work? I can plug that in; most important to me is getting a code comment that says what won't work, but agreed a user message would be nice.

[maxnordlund]

Yeah, but I couldn't really figure out what it turns off. I guess SIGWINCH is for resizing, but what is SIGTSTP used for?

[thomasballinger]

It's backgrounding the process, so with that off I assume ctrl-z will stop working, not positive.

…

On Sat, Apr 22, 2017 at 3:52 PM Max Nordlund ***@***.***> wrote: Yeah, but I couldn't really figure out what it turns off. I guess SIGWINCH is for resizing, but what doe the SIGTSTP do? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#679 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAcAf8UjBniLy4Lw7WW5cRrZOF4t7_MLks5ryoSPgaJpZM4NANXq> .

[maxnordlund]

Yes it would seem that's what they are for, judging from the commit headlines in git blame.

I'll add a comment about that, and this should be good to go?

You can squash the commits if you want to. I even think that's a thing directly on github.

[thomasballinger]

Yep, I'm ok with this! I wonder about some things (will behavior be weird in this state) but you're just making something that didn't work before work. I'll look at a user message later.

[thomasballinger]

Please play around with it a little (not a blocker), since we don't have tests for this signal hehavior, it's just manual testing. Hope it works well for you in Django.

[maxnordlund]

Before I made the pull request I did this on a Django project at work and it did work there. Though I monkey-patched the signal module directly, because it was easier then trying to monkey-patch bpython. Of course that doesn't feel right, hence this PR.

import signal
original = signal.signal

def patched_signal(signalnum, handler):
    try:
        original(signalnum, handler)
    except ValueError:
        pass

setattr(signal, "signal", patched_signal)

[sebastinas]

From what I understand from signal.signals documentation, ValueErrors may also be raised in other cases. So please check if the ValueError really comes from this one specific case.

[thomasballinger]

You might check that we're on the main thread and if not not try since that would be clearer

…

On Sun, Apr 23, 2017 at 2:44 AM Sebastian Ramacher ***@***.***> wrote: From what I understand from signal.signals documentation, ValueErrors may also be raised in other cases. So please check if the ValueError really comes from this one specific case. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#679 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAcAf2Ro4bmR_Ae5PSYWdKzlmsHkX3FEks5ryx1ygaJpZM4NANXq> .

[maxnordlund]

From what I understand from signal.signals documentation, ValueErrors may also be raised in other cases. So please check if the ValueError really comes from this one specific case.

The only other case concerns Windows, https://docs.python.org/3/library/signal.html#signal.signal and since bpython uses SIGWINCH it doesn't work on Windows (probably for other reasons as well).

You might check that we're on the main thread and if not not try since that
would be clearer

I thought it was more pythonic to assume the happy path and catch exceptions if it's not. E.g. not check if a method exists, just try to call it. Follow the duck-typing-way or something like that.

[maxnordlund]

I switched to an if statement, as suggested by @thomasballinger, but there be dragons.

In python 3 we have the threading.main_thread function, but in python 2 you basically have to resort to some form of hack.

@sebastinas I've been thinking about your question a bit more and even if the try-except catches more ValueErrors it's fine because the whole point of this PR is to make sure we don't crash.

[maxnordlund]

Ping @thomasballinger, how do we move forward with this? I personally like the try/except version better, as it feels less hacky. But it's you call.

[maxnordlund]

@thomasballinger ping

[thomasballinger]

I just change Curtsies to be fine with this too, trying it out with this now.

[thomasballinger]

Looks like this needs to happen in coderunner as well

[thomasballinger]

Still to do:

• focus_on_subprocess
• ?

[thomasballinger]

This seems not to be done (at least not in 0.17.1)

[thomasballinger]

hm now it's working for me, not sure what's up

[maxnordlund]

It can be a race condition between when the signal handler is being registered, if it happens to be on the main thread it's fine. It's just when you try to register a handler on some other thread Python complains (and POSIX as well I guess).