Skip to content

thrdpool: retry failed thread starts while items wait - #22303

Closed
GrahamCampbell wants to merge 2 commits into
curl:masterfrom
GrahamCampbell:thrdpool-retry-thread-start
Closed

thrdpool: retry failed thread starts while items wait#22303
GrahamCampbell wants to merge 2 commits into
curl:masterfrom
GrahamCampbell:thrdpool-retry-thread-start

Conversation

@GrahamCampbell

Copy link
Copy Markdown
Contributor

Curl_thrdq_send() queues the item first and cannot report a failed pool signal afterwards, since the queue already owns the item. So when the pool has no worker and Curl_thread_create() fails, nothing ever retries: the item sits in the send queue until the next send comes along and a name resolve strands for up to the 300 second resolve timeout, long after threads became available again. We came across this while investigating a production incident on AWS Lambda, where the 1024-thread cap per environment makes such failures likely under load; the thread pool has since removed the unbounded thread growth that bit us back then, but a failed start there today leaves a resolve stranded instead.

Now Curl_thrdq_recv() retries starting a worker when items wait while the pool has none. Event-based applications do not poll the receive side, so Curl_async_pollset() attempts the same rescue and expires the transfer again after 100ms while the queue stays stalled. A new CURL_DBG_THRDPOOL_FAIL_STARTS debug variable injects start failures for testing: unit 3306 fails without the fix and a new pytest case covers the full resolve path, verified in both curl_multi_perform and --test-event modes.

@github-actions github-actions Bot added the tests label Jul 12, 2026
@GrahamCampbell
GrahamCampbell marked this pull request as ready for review July 12, 2026 20:00
@GrahamCampbell
GrahamCampbell force-pushed the thrdpool-retry-thread-start branch from 8e94cea to f0d8c24 Compare July 12, 2026 21:36
@bagder
bagder requested review from Copilot and icing July 13, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves robustness of the threaded resolver’s work queue/thread pool by retrying worker creation when earlier thread starts failed, preventing queued resolve items from stalling until a subsequent send. It also adds debug injection and tests to reproduce and cover the failure mode seen under thread caps (e.g., AWS Lambda).

Changes:

  • Add “rescue” logic to restart a stalled thread pool from Curl_thrdq_recv() and from the event-based async poll path (Curl_async_pollset()).
  • Introduce CURL_DBG_THRDPOOL_FAIL_STARTS debug env var to inject thread-start failures for testing.
  • Add unit/integration tests validating resolve completion/failure behavior under forced thread-start failures.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/unit3306.c New unit test to reproduce/rescue stalled queue when thread starts initially fail.
tests/unit/Makefile.inc Registers unit3306 in the unit test build.
tests/http/test_21_resolve.py Adds pytest covering the full threaded-resolve path under injected thread-start failures.
tests/data/test3306 New test data enabling the unit test with required debug env var.
tests/data/Makefile.am Registers test3306 in the testcases list.
lib/thrdqueue.h Documents recv-side rescue behavior and adds Curl_thrdq_rescue() API.
lib/thrdqueue.c Implements Curl_thrdq_rescue() and calls it from Curl_thrdq_recv() when no processed items are available.
lib/thrdpool.h Adds Curl_thrdpool_signal_stalled() for “only when no workers exist” signaling.
lib/thrdpool.c Implements stalled-only signaling and adds debug-injected thread start failures.
lib/asyn-thrdd.c In wakeup/event mode, detects stalled resolver queue and expires sooner to retry recovery.
docs/libcurl/libcurl-env-dbg.md Documents CURL_DBG_THRDPOOL_FAIL_STARTS.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/thrdqueue.c Outdated
Comment thread lib/asyn-thrdd.c Outdated
Comment thread lib/thrdpool.c
Comment thread lib/thrdpool.c Outdated
Comment thread lib/thrdqueue.c Outdated
Comment thread lib/thrdqueue.c Outdated
@GrahamCampbell
GrahamCampbell force-pushed the thrdpool-retry-thread-start branch from f0d8c24 to 2e1861e Compare July 13, 2026 12:18
@GrahamCampbell

Copy link
Copy Markdown
Contributor Author

Thanks for the review @icing. Super helpful. I've applied your feedback. 🚀

@icing icing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@GrahamCampbell
GrahamCampbell force-pushed the thrdpool-retry-thread-start branch from cc52b8b to 9e6aa4b Compare July 15, 2026 10:39
@GrahamCampbell

Copy link
Copy Markdown
Contributor Author

Rebased on master. No code changes since the last review.

@GrahamCampbell
GrahamCampbell force-pushed the thrdpool-retry-thread-start branch from 9e6aa4b to f7d61cc Compare July 15, 2026 20:33
@bagder bagder closed this in 4dc236a Jul 21, 2026
@GrahamCampbell
GrahamCampbell deleted the thrdpool-retry-thread-start branch July 21, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants