fix: use shared idle tracker for relay connections#424
Merged
9seconds merged 3 commits into9seconds:masterfrom Mar 30, 2026
Merged
fix: use shared idle tracker for relay connections#4249seconds merged 3 commits into9seconds:masterfrom
9seconds merged 3 commits into9seconds:masterfrom
Conversation
connIdleTimeout previously set per-direction deadlines independently. During media downloads the client→telegram direction can be idle at the application level while telegram→client is actively streaming data. After IdleTimeout (default 1 min) the idle direction's ReadDeadline fires, tearing down the entire relay and breaking media transfers. Replace the per-direction timeout with a shared atomic timestamp that both pump goroutines update on any successful Read or Write. When a ReadDeadline fires on the idle direction, we check the shared tracker: if the other direction was recently active, we retry instead of closing. The connection is only torn down when both directions are idle for the full timeout period. This matches the documented IdleTimeout contract: "if we have any message which will pass to either direction, a timer is reset." Overhead: one atomic.Int64 (8 bytes) per connection pair, one atomic.Store (~1 ns) per Read/Write with data, zero extra goroutines. Fixes 9seconds#423
Owner
|
Да, спасибо. Заняло где-то с минуту понять, в чем же оригинальная проблема, но кажется фикс корректный |
Owner
|
Могу я попросить вас добавить тест в conns_internal_test? |
Cover shared idle tracker behavior: - tracker lifecycle (new, idle after timeout, touch resets) - read/write with data touches tracker - read retries on timeout when tracker is not idle - read closes on timeout when tracker is idle - shared tracker prevents false timeout across directions
Contributor
Author
|
Добавил — |
Owner
|
Спасибо! Я сейчас тогда соберу новый PGO (на горячем пути все-таки) и сделаю релиз |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #423 — media downloads broken in v2.2.5 due to per-direction idle timeout.
connIdleTimeout(introduced in #420) setReadDeadline/WriteDeadlineindependently for each direction. During media downloads, the client→telegram direction is idle at the application level while telegram→client is actively streaming. AfterIdleTimeout(default 1 min), the idle direction'sReadDeadlinefires, which tears down the relay and kills the transfer.The current behavior doesn't match the
IdleTimeoutdocumentation:The actual implementation tracked each direction independently — activity in one direction did not prevent timeout in the other.
Fix
Replace the per-direction timeout with a shared
idleTracker— one atomic timestamp per connection pair. Both pump goroutines update it on any successfulRead/Write. WhenReadDeadlinefires on an idle direction, we check the shared tracker: if the other direction was recently active, retry instead of closing.The connection is torn down only when both directions are idle for the full timeout period — matching the documented contract.
Overhead
atomic.Int64(8 bytes) per connection pairatomic.Store(~1 ns) perRead/Writewith dataTest plan