Register for signals only when running in the main thread

[bmbouter]

Describe the bug
We have a simple pytest fixture that sets up a proxy.py. Here's a copy/paste of it:

import proxy
proxypy_args = ["--num-workers", "4", "--hostname", host, "--port", str(port)]
with proxy.Proxy(input_args=proxypy_args):
    yield

When running it locally it works perfectly. When running it in the Github Actions CI environment it fails ocassionally, but enough that it's a problem. Here's the traceback we have gotten several times:

_____________ ERROR at setup of test_sync_https_through_http_proxy _____________
[gw7] linux -- Python 3.8.12 /opt/hostedtoolcache/Python/3.8.12/x64/bin/python

unused_port = <function unused_port.<locals>._unused_port at 0x7f6295b373a0>

    @pytest.fixture
    def http_proxy(unused_port):
        host = cfg.aiohttp_fixtures_origin
        port = unused_port()
        proxypy_args = [
            "--num-workers",
            "4",
            "--hostname",
            host,
            "--port",
            str(port),
        ]
    
        proxy_data = ProxyData(host=host, port=port)
    
>       with proxy.Proxy(input_args=proxypy_args):

/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/pulp_smash/pulp3/pytest_plugin/__init__.py:164: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/proxy/proxy.py:171: in __enter__
    self.setup()
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/proxy/proxy.py:258: in setup
    self._register_signals()
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/proxy/proxy.py:309: in _register_signals
    signal.signal(signal.SIGINT, self._handle_exit_signal)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

signalnum = <Signals.SIGINT: 2>
handler = <function Proxy._handle_exit_signal at 0x7f629ddd8e50>

    @_wraps(_signal.signal)
    def signal(signalnum, handler):
>       handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
E       ValueError: signal only works in main thread

/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/signal.py:47: ValueError

To Reproduce
I don't have a great reproducer. It's just pytest on github actions.

Version information

• OS: Ubuntu (github issues), but pip installed
• proxy.py==2.4.0

[bmbouter]

The issue here seems to be on proxy shutdown. What I don't understand is what is introducing threading. We use the generic pytest runner so I don't see how it could be threaded. Does proxy.py use threading in how I'm running it?

[bmbouter]

Per the docs, I expected it to be using threadless because it's python3.8+ and linux, but maybe it's not? I'm not specifying --threadless but maybe I should? How can I confirm which mode proxy.py is running in?

[abhinavsingh]

Hi @bmbouter , as you discovered, it will run in --threadless (local) mode by default. Under this mode, it will spawn processes for acceptors. I see you are using --num-workers which has no effect in local executor mode. That flag is application when running in remote executor mode. I know it's confusing, but you must use --num-acceptors to start 4 acceptor processes. Further, each acceptor will spawn a single thread for asyncio request handling.

Per this thread https://bugs.python.org/issue38904, ValueError: signal only works in main thread is a problem starting from Python 3.8. Likely, pytest starts the fixture in a separate thread or something, I am not sure. Irrespective, per that thread, having this guard will help:

if threading.current_thread() is threading.main_thread() then register signals

[abhinavsingh]

You can try it from #1087 branch. I hope this fix works out for you :)

[abhinavsingh]

If you are interested, read about local vs remote modes here.

[abhinavsingh]

Merged. Lemme know if it works. You can try out the pre-releases for quick verification. Happy to release 2.4.1 if this helps you.

[bmbouter]

I'm going to test it now and it'll run in our CI for a while. I'll watch for the issue for a week and let you know. Feel free to cut a 2.4.1 sooner if you want or you can wait to hear back.

You are incredibly responsive, and I really really appreciate it and this proxy. proxy.py is great!

[bmbouter]

@abhinavsingh after a week of observation while our CI uses the commit with the fix, I can confirm we've seen 0 instances of the issue anymore. If you're able to release 2.4.1 that would be much appreciated. Thank you! ❤️

[abhinavsingh]

@bmbouter Thank you, I'll make a release over the weekend. I ran it last night and it failed due to spell check errors :(. Will try over the weekend :)

[bmbouter]

@abhinavsingh is releasing the 2.4.1 possible? Thanks!

[abhinavsingh]

@bmbouter I'll give it a try this week. Apologies for the delay. Something has gone wrong with rst markdown lib due to which our GHA is failing, unable to make new release. I'll have to either fix it or do a manual release.

Give me time till Wednesday. Thank you!!!

[bmbouter]

It's no problem at all. If you have time that would be great. Thank you!

[abhinavsingh]

#1104 and #1107 will help us make automated release again. Looks like it is very important for us to make constant releases. Long delays can cause CI to randomly fail due to upstream build only dependency changes.

We'll release 2.4.1 shortly. Best

[bmbouter]

This is wonderful, thank you!