Repository navigation
ThreadPoolExecutor unbalanced semaphore count #88354
Description
| BPO | 44188 |
|---|---|
| Nosy | @pitrou, @asvetlov, @1st1, @Tinche, @bennieswart |
| PRs | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2021-05-20.11:51:36.159>
labels = ['type-bug', 'library', '3.11']
title = 'ThreadPoolExecutor unbalanced semaphore count'
updated_at = <Date 2021-09-02.23:11:57.890>
user = 'https://github.com/bennieswart'bugs.python.org fields:
activity = <Date 2021-09-02.23:11:57.890>
actor = 'tinchester'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-05-20.11:51:36.159>
creator = 'bennieswart'
dependencies = []
files = ['50054']
hgrepos = []
issue_num = 44188
keywords = ['patch']
message_count = 3.0
messages = ['394017', '400768', '400967']
nosy_count = 5.0
nosy_names = ['pitrou', 'asvetlov', 'yselivanov', 'tinchester', 'bennieswart']
pr_nums = ['26265']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue44188'
versions = ['Python 3.11']
Activity
bennieswart commentedon May 20, 2021
The concurrent.futures.ThreadPoolExecutor class, which is the default asyncio executor, introduced the _idle_semaphore field in version 3.8 in order to track idle threads so they can be reused before increasing the pool size unnecessarily.
This semaphore counter becomes unbalanced when the thread pool is over-saturated, as can be seen in the file provided. This is due to workers always incrementing the count after finishing a job, whereas the executor only decrements the count if it is already greater than 0.
This seems to be a logic bug unrelated to the running environment and introduced since python 3.8.
pitrou commentedon Aug 31, 2021
To be clear, this probably doesn't have any actual consequence, since the right number of threads is launched anyway (at least in the example). But it's probably worth making the implementation less quirky (also, the semaphore's internal counter *might* overflow at some point?).
Tinche commentedon Sep 2, 2021
I was trying to instrument one of our executors to expose some metrics, so I tried using the value of the semaphore as the number of idle threads and I noticed it was way too large. If this was fixed getting these metrics would be easier.