Skip to content

bpo-36780: Add wait_at_exit to ThreadPoolExecutor.shutdown. #13250

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hniksic
Copy link
Contributor

@hniksic hniksic commented May 11, 2019

@@ -211,11 +211,14 @@ def _initializer_failed(self):
if work_item is not None:
work_item.future.set_exception(BrokenThreadPool(self._broken))

def shutdown(self, wait=True):
def shutdown(self, wait=True, wait_at_exit=True):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shutdown is defined in Executor because Executor is the abstract superclass for both ThreadPoolExecutor and ProcessPoolExecutor. Unless there is a very strong reason not to, this method should work the same in both executors.

Copy link
Contributor Author

@hniksic hniksic May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brianquinlan This was intentional - I tested shutdown(wait=False) with ProcessPoolExecutor, and found that it raised exceptions and hanged the process at exit. (Not just hanged in the sense of waiting for the pending futures, but completely hanged, even when the futures exited.) So the new functionality is only available in and documented for ThreadPoolExecutor.

For example, when I run this script on Python 3.7:

import time, concurrent.futures

pool = concurrent.futures.ProcessPoolExecutor()

pool.submit(time.sleep, 5)
print(1)

pool.shutdown(wait=False)
print(2)

The expected behavior is for the program to print 1 and 2 and then to wait for 5 seconds before exiting. Instead, it prints 1 and 2, but hangs at exit with the following output:

$ python3.7 ~/Desktop/x
1
2
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/usr/lib/python3.7/concurrent/futures/process.py", line 101, in _python_exit
    thread_wakeup.wakeup()
  File "/usr/lib/python3.7/concurrent/futures/process.py", line 89, in wakeup
    self._writer.send_bytes(b"")
  File "/usr/lib/python3.7/multiprocessing/connection.py", line 183, in send_bytes
    self._check_closed()
  File "/usr/lib/python3.7/multiprocessing/connection.py", line 136, in _check_closed
    raise OSError("handle is closed")
Exception in thread QueueManagerThread:
Traceback (most recent call last):
  File "/usr/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3.7/concurrent/futures/process.py", line 368, in _queue_management_worker
    thread_wakeup.clear()
  File "/usr/lib/python3.7/concurrent/futures/process.py", line 92, in clear
    while self._reader.poll():
  File "/usr/lib/python3.7/multiprocessing/connection.py", line 255, in poll
    self._check_closed()
  File "/usr/lib/python3.7/multiprocessing/connection.py", line 136, in _check_closed
    raise OSError("handle is closed")
OSError: handle is closed
OSError: handle is closed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really wanted ThreadPoolExecutor and ProcessPoolExecutor to have the same API when I designed them.

Do you have any bandwidth to debug this? If not, I could take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do take a look if you can. I am not acquainted with the implementation of ProcessPoolExecutor, so it would take quite some time for me to trace what's going on.

It would of course be ideal if both classes supported the new flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was playing with ProcessPoolExecutor and it seems like there are a bunch of problems that are triggered when pool.shutdown(wait=False) is used. I filed a bug for one issue: https://bugs.python.org/issue39205

Do you think that your PR could hold off until I have a chance to sort some of this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that your PR could hold off until I have a chance to sort some of this out?

Sure, thanks for asking. We have a workaround, so it's no problem to wait for the proper solution. It's just that the workaround is so extremely ugly, involving monkey patch of a private method, that we'd definitely prefer the proper fix to land eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants