Skip to content
This repository has been archived by the owner on Feb 10, 2023. It is now read-only.

Fix thread counter leak #28

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

amosbird
Copy link

This issue has appeared three times in kuaishou (every 1-2 months). TCPServerDispatcher fails to create any new threads because _currentThreads is leaked.

The issue is introduced in pocoproject#1965 which was actual to fix another _currentThreads leaking issue... However I cannot find the related commit in current poco's master or develop branch. Perhaps it's already been reverted.

@alexey-milovidov
Copy link
Member

CC @akuzm

@amosbird
Copy link
Author

I've successfully reproduced the issue with gdb. Here are the steps:

  1. Set a breakpoint at https://github.com/ClickHouse-Extras/poco/blob/clickhouse/Net/src/TCPServerDispatcher.cpp#L105
  2. Submit three queries select 1 via HTTP. Now we have three suspended threads at the above breakpoint. Let's denote them as A, B and C.
  3. Step both A and B right before https://github.com/ClickHouse-Extras/poco/blob/clickhouse/Net/src/TCPServerDispatcher.cpp#L123 , that is, we successfully destruct ThreadCountWatcher but without decrementing _currentThreads because the _queue is not empty.
  4. Step C to dequeue the last query. Now we have an empty queue.
  5. Continue both A and B. They'll break the main loop and exit (in the sense of Poco, they'll be cached)

Now we have _currentThreads = 3 but there is only one thread left.

@alexey-milovidov
Copy link
Member

Sorry, we almost missed this PR.

@alexey-milovidov alexey-milovidov merged commit f3d791f into ClickHouse:clickhouse Nov 26, 2020
alesapin pushed a commit that referenced this pull request Dec 7, 2020
Fix thread counter leak

(cherry picked from commit f3d791f)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants