Skip to content

HTTP#close waits for the threads it kills - #49

Merged
PetrHeinz merged 3 commits into
mainfrom
claude/http-close-joins-threads
Sep 30, 2026
Merged

PetrHeinz merged 3 commits into
mainfrom
claude/http-close-joins-threads

Conversation

@PetrHeinz

Copy link
Copy Markdown
Member

Logtail::LogDevices::HTTP#close should kill the threads failed on TruffleRuby in https://github.com/logtail/logtail-ruby/actions/runs/36743953257/job/109985245554 with the flush thread still alive? and "aborting". close only calls Thread#kill, which asks the threads to stop and returns at once, so the example slept 0.1 s and hoped for the best (its own comment: "too fast!").

The first commit only removes the sleeps and is expected to fail on every job: on Ruby 3.4 it fails 30 out of 30 runs locally. The second commit makes close join the threads it kills, so it returns only once they are gone, and the example passes without waiting. This is also what a caller of close should get, Logtail::Logger calls it from at_exit.

🤖 Generated with Claude Code

PetrHeinz and others added 3 commits September 30, 2026 18:31
The example slept 0.1 s after close and was still flaky on TruffleRuby,
because Thread#kill only asks the thread to stop. Without the sleeps it
fails on every run: close returns while the threads are still winding
down.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Thread#kill only asks a thread to stop, so close returned while the
flush and request threads were still winding down, which made the close
example flaky on TruffleRuby. Joining them makes close return once they
are gone, which is what its at_exit caller should get.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@PetrHeinz
PetrHeinz marked this pull request as ready for review September 30, 2026 16:33
@PetrHeinz
PetrHeinz merged commit d90e0c3 into main Sep 30, 2026
13 checks passed
@PetrHeinz
PetrHeinz deleted the claude/http-close-joins-threads branch September 30, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant