Bug Fix: Fixed a provider TCP connection leak in the VOD proxy

This commit is contained in:
SergeantPanda 2026-04-13 19:33:22 -05:00
parent a3f7d465d5
commit f98bb183da
2 changed files with 18 additions and 0 deletions

View file

@ -50,6 +50,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Fixed two race conditions in the VOD proxy that caused the `profile_connections` counter to go permanently negative, allowing connections beyond the configured profile limit. (1) `_decrement_profile_connections()` used a GET-before-DECR guard: two concurrent decrements could both read the same positive value, both pass the guard, and both fire, driving the counter below zero. Replaced with an unconditional `DECR` followed by a clamp-to-zero if the result is negative. (2) The `stream_generator` decremented `active_streams` and then checked `has_active_streams()` in two separate Redis round-trips without locking. A concurrent generator on another worker could read `active_streams=0` in the window between those two calls and also decrement the profile counter, producing a double-decrement. A new `decrement_active_streams_and_check()` method performs both operations under a single distributed lock, and a `profile_decremented` flag guards all four call sites in the generator so the profile counter is only ever decremented once per stream. (Closes #1125) — Thanks [@firestaerter3](https://github.com/firestaerter3)
- Fixed a provider TCP connection leak in the VOD proxy `stream_generator`. When a stream ended via an unhandled exception path that reached the `finally` block without any of the three exception handlers having run (e.g. an error raised before the first `yield`), the `finally` block decremented counters but never called `redis_connection.cleanup()`. The upstream `requests.Response` and `requests.Session` were left open until garbage collection. The `finally` block now starts a `delayed_cleanup` daemon thread (matching the 1-second delay used by the normal-completion and `GeneratorExit` paths) so that seeking clients have time to reconnect and increment `active_streams` before `cleanup()` checks whether it is safe to close the connection.
- Fixed manual stream selection from the Stats page not enforcing M3U profile connection limits in multi-worker deployments. When a non-owning worker handled the `change_stream` request it correctly packaged `stream_id` and `m3u_profile_id` into the Redis pubsub message, but the owning worker's pubsub handler only consumed `url` and `user_agent` silently dropping both IDs before calling `stream_manager.update_url()`. Because `update_url` only calls `update_stream_profile()` when a `stream_id` is provided, the `profile_connections` counter was never updated after the switch, causing subsequent capacity checks to see incorrect counts and bypass the full-profile guard. The handler now extracts `stream_id` and `m3u_profile_id` from the event and forwards them to `update_url()`. The bug did not affect single-worker / dev-mode deployments because the owning worker handles those requests directly without pubsub.
- Fixed the `next_stream` rotation endpoint applying the same class of bug: `get_stream_info_for_switch()` was called and returned `m3u_profile_id`, but the result was dropped when forwarding to `ChannelService.change_stream_url()`, so `update_stream_profile()` was never called and `profile_connections` counters were not updated after an automatic stream rotation.
- Fixed stream switch metadata (`url`, `user_agent`, `stream_id`, `m3u_profile`) being written to Redis before the switch was confirmed to succeed. If the switch failed, URL unchanged or exception during teardown, Redis described a URL not actually in use. Metadata is now written only after `update_url()` returns `True`; on failure the owner writes `stream_manager.url` back as the ground truth. The non-owner no longer pre-writes metadata at all, all needed info is carried in the pubsub payload and written by the owner after confirmation.

View file

@ -1131,6 +1131,22 @@ class MultiWorkerVODConnectionManager:
profile_decremented = True
logger.info(f"[{client_id}] Profile counter decremented for profile {profile_id} in finally block")
# Delayed cleanup: wait 1s for seeking clients to reconnect
# before closing the provider connection and Redis keys.
# cleanup() re-checks active_streams under lock, so a
# reconnecting client that increments active_streams in
# time will prevent Redis key deletion.
def delayed_cleanup():
time.sleep(1)
logger.info(f"[{client_id}] Worker {self.worker_id} - Checking for smart cleanup in finally block")
# No connection_manager — profile already decremented above
redis_connection.cleanup(current_worker_id=self.worker_id)
import threading
cleanup_thread = threading.Thread(target=delayed_cleanup)
cleanup_thread.daemon = True
cleanup_thread.start()
# Create streaming response
response = StreamingHttpResponse(
streaming_content=stream_generator(),