Skip to content
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

[3.9] bpo-37193: remove thread objects which finished process its request (GH-13893) #23087

Closed

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Nov 1, 2020

  • bpo-37193: remove the thread which finished process request from threads list

  • rename variable t to thread.

  • don't remove thread from list if it is daemon.

  • use lock to protect self._threads.

  • use finally block in case of exception from shutdown_request().

  • check "not thread.daemon" before lock to avoid holding the lock if it's unnecessary.

  • fix the place of _threads_lock.

  • separate code to remove a current thread into a function.

  • check ValueError when removing thread.

  • fix wrong code which all instance shared same lock.

  • Extract thread management into a _Threads class to encapsulate atomic operations and separate concerns.

  • Replace multiple references of 'block_on_close' with one, avoiding the possibility that 'block_on_close' could change during the course of processing requests. Now, there's exactly one _threads object with behavior fixed for the duration.

  • Add docstrings to private classes.

  • Add test to ensure that a ThreadingTCPServer can be closed without serving any requests.

  • Use _NoThreads as the default value. Fixes AttributeError when server is closed without serving any requests.

  • Add blurb

  • Add test capturing failure.

Co-authored-by: Jason R. Coombs jaraco@jaraco.com
(cherry picked from commit c415590)

Co-authored-by: MARUYAMA Norihiro norihiro.maruyama@gmail.com

https://bugs.python.org/issue37193

…ythonGH-13893)

* bpo-37193: remove the thread which finished process request from threads list

* rename variable t to thread.

* don't remove thread from list if it is daemon.

* use lock to protect self._threads.

* use finally block in case of exception from shutdown_request().

* check "not thread.daemon" before lock to avoid holding the lock if it's unnecessary.

* fix the place of _threads_lock.

* separate code to remove a current thread into a function.

* check ValueError when removing thread.

* fix wrong code which all instance shared same lock.

* Extract thread management into a _Threads class to encapsulate atomic operations and separate concerns.

* Replace multiple references of 'block_on_close' with one, avoiding the possibility that 'block_on_close' could change during the course of processing requests. Now, there's exactly one _threads object with behavior fixed for the duration.

* Add docstrings to private classes.

* Add test to ensure that a ThreadingTCPServer can be closed without serving any requests.

* Use _NoThreads as the default value. Fixes AttributeError when server is closed without serving any requests.

* Add blurb

* Add test capturing failure.

Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
(cherry picked from commit c415590)

Co-authored-by: MARUYAMA Norihiro <norihiro.maruyama@gmail.com>
@miss-islington
Copy link
Contributor Author

@maru-n and @jaraco: Status check is done, and it's a success ✅ .

@miss-islington
Copy link
Contributor Author

@maru-n and @jaraco: Status check is done, and it's a success ✅ .

@vstinner
Copy link
Member

vstinner commented Nov 2, 2020

I reject the backport since it causes a regression on master: https://bugs.python.org/issue37193#msg380172

@vstinner vstinner closed this Nov 2, 2020
@miss-islington miss-islington deleted the backport-c415590-3.9 branch November 2, 2020 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants