ThreadPoolExecutor unbalanced semaphore count

[c5c3c9e4-4850-48fb-84fe-432a8895b392]

BPO	44188
Nosy	@pitrou, @asvetlov, @1st1, @Tinche, @bennieswart
PRs	python/cpython#26265
Files	bug.py

^{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: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.11'] title = 'ThreadPoolExecutor unbalanced semaphore count' updated_at = user = 'https://github.com/bennieswart' ``` bugs.python.org fields: ```python activity = actor = 'tinchester' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = 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'] ```

[c5c3c9e4-4850-48fb-84fe-432a8895b392]

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]

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?).

[b656d977-2b3d-484b-b318-8209d3128c21]

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.