From 49b7b9e21b0dd896764d9d12abb61983bd7896e9 Mon Sep 17 00:00:00 2001 From: None Date: Thu, 5 Feb 2026 23:12:30 -0600 Subject: [PATCH 01/58] Fix: Connection capacity leak - failed stream initialization permanently consumes connection slot Fixed four leak paths in the TS proxy streaming initialization: 1. url_utils.py - generate_stream_url(): Wrapped post-get_stream() code in try/except that calls release_stream() on any failure 2. views.py - stream_ts() retry loop: Added release_stream() before returning 503 to clean up the error-checking get_stream() call at line 181 which could INCR without a matching release when the retry loop times out 3. views.py - stream_ts() init failure: Added guarded release_stream() when initialize_channel() returns False, using a connection_allocated flag to prevent incorrect decrements when joining an existing channel 4. views.py - stream_ts() outer except: Added guarded release_stream() as a safety net for unexpected exceptions, only releasing when the flag confirms the slot was allocated and the channel hasn't been initialized yet The connection_allocated flag prevents double-decrements by tracking whether request flow actually INCR'd the counter (fresh initialization) vs returned cached results (joining an existing channel on another worker) --- apps/proxy/ts_proxy/url_utils.py | 60 +++++++++++++++++--------------- apps/proxy/ts_proxy/views.py | 23 ++++++++++-- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/apps/proxy/ts_proxy/url_utils.py b/apps/proxy/ts_proxy/url_utils.py index 8b467b7f..9daaf4ef 100644 --- a/apps/proxy/ts_proxy/url_utils.py +++ b/apps/proxy/ts_proxy/url_utils.py @@ -129,39 +129,41 @@ def generate_stream_url(channel_id: str) -> Tuple[str, str, bool, Optional[int]] logger.error(f"No stream available for channel {channel_id}: {error_reason}") return None, None, False, None - # Look up the Stream and Profile objects + # get_stream() allocated a connection slot - ensure it's released on any error try: + # Look up the Stream and Profile objects stream = Stream.objects.get(id=stream_id) profile = M3UAccountProfile.objects.get(id=profile_id) - except (Stream.DoesNotExist, M3UAccountProfile.DoesNotExist) as e: - logger.error(f"Error getting stream or profile: {e}") + + # Get the M3U account profile for URL pattern + m3u_profile = profile + + # Get the appropriate user agent + m3u_account = M3UAccount.objects.get(id=m3u_profile.m3u_account.id) + stream_user_agent = m3u_account.get_user_agent().user_agent + + if stream_user_agent is None: + stream_user_agent = UserAgent.objects.get(id=CoreSettings.get_default_user_agent_id()) + logger.debug(f"No user agent found for account, using default: {stream_user_agent}") + + # Generate stream URL based on the selected profile + input_url = stream.url + stream_url = transform_url(input_url, m3u_profile.search_pattern, m3u_profile.replace_pattern) + + # Check if transcoding is needed + stream_profile = channel.get_stream_profile() + if stream_profile.is_proxy() or stream_profile is None: + transcode = False + else: + transcode = True + + stream_profile_id = stream_profile.id + + return stream_url, stream_user_agent, transcode, stream_profile_id + except Exception as e: + logger.error(f"Error generating stream URL for channel {channel_id}: {e}") + channel.release_stream() return None, None, False, None - - # Get the M3U account profile for URL pattern - m3u_profile = profile - - # Get the appropriate user agent - m3u_account = M3UAccount.objects.get(id=m3u_profile.m3u_account.id) - stream_user_agent = m3u_account.get_user_agent().user_agent - - if stream_user_agent is None: - stream_user_agent = UserAgent.objects.get(id=CoreSettings.get_default_user_agent_id()) - logger.debug(f"No user agent found for account, using default: {stream_user_agent}") - - # Generate stream URL based on the selected profile - input_url = stream.url - stream_url = transform_url(input_url, m3u_profile.search_pattern, m3u_profile.replace_pattern) - - # Check if transcoding is needed - stream_profile = channel.get_stream_profile() - if stream_profile.is_proxy() or stream_profile is None: - transcode = False - else: - transcode = True - - stream_profile_id = stream_profile.id - - return stream_url, stream_user_agent, transcode, stream_profile_id except Exception as e: logger.error(f"Error generating stream URL: {e}") return None, None, False, None diff --git a/apps/proxy/ts_proxy/views.py b/apps/proxy/ts_proxy/views.py index 91f254a7..6980f957 100644 --- a/apps/proxy/ts_proxy/views.py +++ b/apps/proxy/ts_proxy/views.py @@ -54,6 +54,7 @@ def stream_ts(request, channel_id): client_user_agent = None proxy_server = ProxyServer.get_instance() + connection_allocated = False # Track if connection slot was allocated via get_stream() try: # Generate a unique client ID @@ -219,9 +220,9 @@ def stream_ts(request, channel_id): ) if stream_url is None: - # Release the channel's stream lock if one was acquired - # Note: Only call this if get_stream() actually assigned a stream - # In our case, if stream_url is None, no stream was ever assigned, so don't release + # Release any connection slot that may have been allocated + # by the error-checking get_stream() call during retries + channel.release_stream() # Get the specific error message if available wait_duration = f"{int(time.time() - wait_start_time)}s" @@ -237,6 +238,11 @@ def stream_ts(request, channel_id): {"error": error_msg, "waited": wait_duration}, status=503 ) # 503 Service Unavailable is appropriate here + # generate_stream_url() called get_stream() which allocated a connection + # slot (INCR'd profile_connections) - track this for cleanup on error + if needs_initialization: + connection_allocated = True + # Get the stream ID from the channel stream_id, m3u_profile_id, _ = channel.get_stream() logger.info( @@ -344,10 +350,16 @@ def stream_ts(request, channel_id): ) if not success: + if connection_allocated: + channel.release_stream() + connection_allocated = False return JsonResponse( {"error": "Failed to initialize channel"}, status=500 ) + # Channel initialized - cleanup lifecycle now owns the connection release + connection_allocated = False + # If we're the owner, wait for connection to establish if proxy_server.am_i_owner(channel_id): manager = proxy_server.stream_managers.get(channel_id) @@ -507,6 +519,11 @@ def stream_ts(request, channel_id): except Exception as e: logger.error(f"Error in stream_ts: {e}", exc_info=True) + if connection_allocated: + try: + channel.release_stream() + except Exception: + pass return JsonResponse({"error": str(e)}, status=500) From 16309cd9b455daf98fd1bc5af213c83531111ea9 Mon Sep 17 00:00:00 2001 From: None Date: Tue, 10 Feb 2026 18:15:48 -0600 Subject: [PATCH 02/58] Remove if logic from scheduled shutdown - stream_generator.py: _schedule_channel_shutdown_if_needed() already guards itself, the if logic created conflict between release_stream() and stop_channel(), so if the stream was already released the shutdown would not be scheduled --- apps/proxy/ts_proxy/stream_generator.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 50404f1d..771b0aa9 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -472,8 +472,7 @@ class StreamGenerator: logger.error(f"Could not log client disconnect event: {e}") # Schedule channel shutdown if no clients left - if not stream_released: # Only if we haven't already released the stream - self._schedule_channel_shutdown_if_needed(local_clients) + self._schedule_channel_shutdown_if_needed(local_clients) def _schedule_channel_shutdown_if_needed(self, local_clients): """ From 15d67c7f6518cd599823c57d070562da792f23d2 Mon Sep 17 00:00:00 2001 From: None Date: Tue, 10 Feb 2026 18:30:43 -0600 Subject: [PATCH 03/58] Atomize profile counter swap in update_stream_profile to prevent counter drift DECR/SET/INCR during stream profile switches are now executed in a Redis pipeline, preventing partial updates if an exception occurs mid-operation --- apps/channels/models.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index ec5e135b..dd67095c 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -566,18 +566,18 @@ class Channel(models.Model): if current_profile_id == new_profile_id: return True - # Decrement connection count for old profile + # Use pipeline for atomic profile switch to prevent counter drift + # if an exception occurs between DECR and INCR old_profile_connections_key = f"profile_connections:{current_profile_id}" - old_count = int(redis_client.get(old_profile_connections_key) or 0) - if old_count > 0: - redis_client.decr(old_profile_connections_key) - - # Update the profile mapping - redis_client.set(f"stream_profile:{stream_id}", new_profile_id) - - # Increment connection count for new profile new_profile_connections_key = f"profile_connections:{new_profile_id}" - redis_client.incr(new_profile_connections_key) + old_count = int(redis_client.get(old_profile_connections_key) or 0) + + pipe = redis_client.pipeline() + if old_count > 0: + pipe.decr(old_profile_connections_key) + pipe.set(f"stream_profile:{stream_id}", new_profile_id) + pipe.incr(new_profile_connections_key) + pipe.execute() logger.info( f"Updated stream {stream_id} profile from {current_profile_id} to {new_profile_id}" ) From 9e8f227f494eb0d1aa27b1359d8b470221a56e3a Mon Sep 17 00:00:00 2001 From: None Date: Tue, 10 Feb 2026 18:58:17 -0600 Subject: [PATCH 04/58] Fix: VOD proxy double-DECR of profile connection counter during cleanup - multi_worker_connection_manager.py: Error handler at line 1042: removed connection_manager=self from cleanup() call since the DECR was already done at line 1036. Prevents double-DECR. - connection_manager.py: Added connection_key_exists check before calling remove_connection(). The fallback DECR from session data now only fires when the connection tracking key has already expired. Prevents double-DECR. --- apps/proxy/vod_proxy/connection_manager.py | 22 +++++++++++++------ .../multi_worker_connection_manager.py | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/apps/proxy/vod_proxy/connection_manager.py b/apps/proxy/vod_proxy/connection_manager.py index aafc75cd..42d69a11 100644 --- a/apps/proxy/vod_proxy/connection_manager.py +++ b/apps/proxy/vod_proxy/connection_manager.py @@ -1363,17 +1363,25 @@ class VODConnectionManager: client_id = session_id # Remove individual connection tracking keys created during streaming + # Check if connection key exists first - remove_connection() + # handles the profile DECR when the key exists, but skips it + # if the key already expired by TTL + connection_key_exists = False if content_type and content_uuid: + connection_key = self._get_connection_key(content_type, content_uuid, client_id) + connection_key_exists = bool(self.redis_client.exists(connection_key)) logger.info(f"[{session_id}] Cleaning up connection tracking keys") self.remove_connection(content_type, content_uuid, client_id) - # Remove from profile connections if counted (additional safety check) - if session_data.get(b'connection_counted') == b'True' and profile_id: - profile_key = self._get_profile_connections_key(int(profile_id.decode('utf-8'))) - current_count = int(self.redis_client.get(profile_key) or 0) - if current_count > 0: - self.redis_client.decr(profile_key) - logger.info(f"[{session_id}] Decremented profile {profile_id.decode('utf-8')} connections") + # Fallback DECR: only if the connection tracking key had already + # expired (remove_connection skips DECR in that case) + if not connection_key_exists: + if session_data.get(b'connection_counted') == b'True' and profile_id: + profile_key = self._get_profile_connections_key(int(profile_id.decode('utf-8'))) + current_count = int(self.redis_client.get(profile_key) or 0) + if current_count > 0: + self.redis_client.decr(profile_key) + logger.info(f"[{session_id}] Decremented profile {profile_id.decode('utf-8')} connections (fallback)") # Remove session tracking key self.redis_client.delete(session_key) diff --git a/apps/proxy/vod_proxy/multi_worker_connection_manager.py b/apps/proxy/vod_proxy/multi_worker_connection_manager.py index 1534f761..58d6bd04 100644 --- a/apps/proxy/vod_proxy/multi_worker_connection_manager.py +++ b/apps/proxy/vod_proxy/multi_worker_connection_manager.py @@ -1036,9 +1036,10 @@ class MultiWorkerVODConnectionManager: self._decrement_profile_connections(m3u_profile.id) # Also clean up the Redis connection state since we won't be using it + # Pass connection_manager=None since we already decremented above if redis_connection: try: - redis_connection.cleanup(connection_manager=self, current_worker_id=self.worker_id) + redis_connection.cleanup(current_worker_id=self.worker_id) except Exception as cleanup_error: logger.error(f"[{client_id}] Error during cleanup after connection failure: {cleanup_error}") From cf96287c90a32120ad22aa7a17973ddae96a627a Mon Sep 17 00:00:00 2001 From: None Date: Tue, 10 Feb 2026 19:06:51 -0600 Subject: [PATCH 05/58] Add guard to remove_connection() - multi_worker_connection_manager.py: Add missing count > 0 guard to multi-worker remove_connection() This method doesn't appear to be called anywhere but added the guard in case it is put back into service --- apps/proxy/vod_proxy/multi_worker_connection_manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/proxy/vod_proxy/multi_worker_connection_manager.py b/apps/proxy/vod_proxy/multi_worker_connection_manager.py index 58d6bd04..63eeae00 100644 --- a/apps/proxy/vod_proxy/multi_worker_connection_manager.py +++ b/apps/proxy/vod_proxy/multi_worker_connection_manager.py @@ -1263,12 +1263,14 @@ class MultiWorkerVODConnectionManager: profile_id = connection_data.get('m3u_profile_id') if profile_id: profile_connections_key = f"profile_connections:{profile_id}" + current_count = int(self.redis_client.get(profile_connections_key) or 0) # Use pipeline for atomic operations pipe = self.redis_client.pipeline() pipe.delete(connection_key) pipe.srem(content_connections_key, client_id) - pipe.decr(profile_connections_key) + if current_count > 0: + pipe.decr(profile_connections_key) pipe.execute() logger.info(f"Removed Redis-backed connection: {client_id}") From 5a4b51b60a90d64d11a56df273607cc0be6ac72d Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 18:08:24 -0600 Subject: [PATCH 06/58] Fix TOCTOU race condition in Channel.get_stream() with atomic slot reservation Add _check_and_reserve_profile_slot() method that uses an INCR-first-then-check pattern to atomically reserve connection slots. This eliminates the TOCTOU race condition where separate GET > check > INCR operations allowed concurrent requests to both pass the capacity check, exceeding max_streams. For profiles with max_streams=0 (unlimited), INCR is skipped entirely. If over capacity, the increment is rolled back with DECR. Based on PR #838 by patchy8736, adapted from Lua script to native Redis commands for consistency with project style and easier debugging. --- apps/channels/models.py | 53 ++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index dd67095c..50538238 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -391,6 +391,38 @@ class Channel(models.Model): return stream_profile + def _check_and_reserve_profile_slot(self, profile, redis_client): + """ + Atomically check and reserve a connection slot for the given profile. + + Uses an INCR-first-then-check pattern to eliminate the TOCTOU race + condition where separate GET + check + INCR operations could allow + concurrent requests to both pass the capacity check. + + For profiles with max_streams=0 (unlimited), no reservation is needed. + + Args: + profile: M3UAccountProfile instance + redis_client: Redis client instance + + Returns: + tuple: (reserved: bool, current_count: int) + """ + if profile.max_streams == 0: + return (True, 0) + + profile_connections_key = f"profile_connections:{profile.id}" + + # Atomically increment first — this is a single Redis command + new_count = redis_client.incr(profile_connections_key) + + if new_count <= profile.max_streams: + return (True, new_count) + + # Over capacity — roll back the increment + redis_client.decr(profile_connections_key) + return (False, new_count - 1) + def get_stream(self): """ Finds an available stream for the requested channel and returns the selected stream and profile. @@ -456,24 +488,16 @@ class Channel(models.Model): for profile in profiles: has_active_profiles = True - profile_connections_key = f"profile_connections:{profile.id}" - current_connections = int( - redis_client.get(profile_connections_key) or 0 + # Atomically check and reserve a slot (INCR-first pattern) + reserved, current_count = self._check_and_reserve_profile_slot( + profile, redis_client ) - # Check if profile has available slots (or unlimited connections) - if ( - profile.max_streams == 0 - or current_connections < profile.max_streams - ): - # Start a new stream + if reserved: + # Slot reserved — assign stream to this channel redis_client.set(f"channel_stream:{self.id}", stream.id) redis_client.set(f"stream_profile:{stream.id}", profile.id) - # Increment connection count for profiles with limits - if profile.max_streams > 0: - redis_client.incr(profile_connections_key) - return ( stream.id, profile.id, @@ -483,7 +507,8 @@ class Channel(models.Model): # This profile is at max connections has_streams_but_maxed_out = True logger.debug( - f"Profile {profile.id} at max connections: {current_connections}/{profile.max_streams}" + f"Profile {profile.id} at max connections: " + f"{current_count}/{profile.max_streams}" ) # No available streams - determine specific reason From 33f68a9808a1a29eed16afb6d85ab8b04ff58c85 Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 19:11:28 -0600 Subject: [PATCH 07/58] Remove unused Stream.get_stream() with TOCTOU vulnerability Remove the Stream.get_stream() method which had the same TOCTOU race condition as Channel.get_stream() All stream acquisition goes through Channel.get_stream(), making this dead code. Removing it eliminates a potential source of connection capacity leaks Based on PR #838 by patchy8736 --- apps/channels/models.py | 48 ----------------------------------------- 1 file changed, 48 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index 50538238..998b7aca 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -202,54 +202,6 @@ class Stream(models.Model): return stream_profile - def get_stream(self): - """ - Finds an available stream for the requested channel and returns the selected stream and profile. - """ - redis_client = RedisClient.get_client() - profile_id = redis_client.get(f"stream_profile:{self.id}") - if profile_id: - profile_id = int(profile_id) - return self.id, profile_id, None - - # Retrieve the M3U account associated with the stream. - m3u_account = self.m3u_account - m3u_profiles = m3u_account.profiles.all() - default_profile = next((obj for obj in m3u_profiles if obj.is_default), None) - profiles = [default_profile] + [ - obj for obj in m3u_profiles if not obj.is_default - ] - - for profile in profiles: - logger.info(profile) - # Skip inactive profiles - if profile.is_active == False: - continue - - profile_connections_key = f"profile_connections:{profile.id}" - current_connections = int(redis_client.get(profile_connections_key) or 0) - - # Check if profile has available slots (or unlimited connections) - if profile.max_streams == 0 or current_connections < profile.max_streams: - # Start a new stream - redis_client.set(f"channel_stream:{self.id}", self.id) - redis_client.set( - f"stream_profile:{self.id}", profile.id - ) # Store only the matched profile - - # Increment connection count for profiles with limits - if profile.max_streams > 0: - redis_client.incr(profile_connections_key) - - return ( - self.id, - profile.id, - None, - ) # Return newly assigned stream and matched profile - - # 4. No available streams - return None, None, None - def release_stream(self): """ Called when a stream is finished to release the lock. From 56719d93fe53445226a8c2dd812e9a23287f2ced Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 19:16:01 -0600 Subject: [PATCH 08/58] Add boolean return and metadata fallback to Channel.release_stream() Channel.release_stream() now returns True on success and False when no stream/profile info could be found for cleanup. This allows callers to detect and handle failed releases. When the primary Redis keys (channel_stream/stream_profile) are already cleaned up by the proxy, the method falls back to the channel metadata hash (ts_proxy:channel:{uuid}:metadata) to recover stream_id and profile_id for proper connection counter cleanup. Uses str(self.uuid) for metadata key lookup - fixes a bug in PR #838 which used self.id (integer PK), but metadata is keyed by UUID. Enhanced logging now includes channel UUID for better traceability. Based on PR #838 by patchy8736 --- apps/channels/models.py | 58 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index 998b7aca..9e466cc7 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -3,6 +3,8 @@ from django.core.exceptions import ValidationError from django.conf import settings from core.models import StreamProfile, CoreSettings from core.utils import RedisClient +from apps.proxy.ts_proxy.redis_keys import RedisKeys +from apps.proxy.ts_proxy.constants import ChannelMetadataField import logging import uuid from datetime import datetime @@ -476,32 +478,74 @@ class Channel(models.Model): def release_stream(self): """ Called when a stream is finished to release the lock. + + Returns: + bool: True if stream was successfully released, False if + no stream/profile info could be found for cleanup. """ redis_client = RedisClient.get_client() stream_id = redis_client.get(f"channel_stream:{self.id}") if not stream_id: - logger.debug("Invalid stream ID pulled from channel index") - return + # Primary key missing — try metadata hash fallback. + # The proxy may have already cleaned up channel_stream/stream_profile + # keys, but the metadata hash can still have the stream_id and profile. + metadata_key = RedisKeys.channel_metadata(str(self.uuid)) + meta_stream_id = redis_client.hget( + metadata_key, ChannelMetadataField.STREAM_ID + ) + meta_profile_id = redis_client.hget( + metadata_key, ChannelMetadataField.M3U_PROFILE + ) + + if meta_stream_id and meta_profile_id: + stream_id = int(meta_stream_id) + profile_id = int(meta_profile_id) + logger.debug( + f"Channel {self.uuid}: recovered stream_id={stream_id}, " + f"profile_id={profile_id} from metadata fallback" + ) + # Clean up any remaining keys + redis_client.delete(f"channel_stream:{self.id}") + redis_client.delete(f"stream_profile:{stream_id}") + + profile_connections_key = f"profile_connections:{profile_id}" + current_count = int( + redis_client.get(profile_connections_key) or 0 + ) + if current_count > 0: + redis_client.decr(profile_connections_key) + return True + + logger.debug( + f"Channel {self.uuid}: no stream info found in primary keys " + f"or metadata fallback" + ) + return False redis_client.delete(f"channel_stream:{self.id}") # Remove active stream stream_id = int(stream_id) logger.debug( - f"Found stream ID {stream_id} associated with channel stream {self.id}" + f"Channel {self.uuid}: found stream_id={stream_id} for " + f"channel_stream:{self.id}" ) # Get the matched profile for cleanup profile_id = redis_client.get(f"stream_profile:{stream_id}") if not profile_id: - logger.debug("Invalid profile ID pulled from stream index") - return + logger.debug( + f"Channel {self.uuid}: no profile found for " + f"stream_profile:{stream_id}" + ) + return False redis_client.delete(f"stream_profile:{stream_id}") # Remove profile association profile_id = int(profile_id) logger.debug( - f"Found profile ID {profile_id} associated with stream {stream_id}" + f"Channel {self.uuid}: found profile_id={profile_id} for " + f"stream {stream_id}" ) profile_connections_key = f"profile_connections:{profile_id}" @@ -511,6 +555,8 @@ class Channel(models.Model): if current_count > 0: redis_client.decr(profile_connections_key) + return True + def update_stream_profile(self, new_profile_id): """ Updates the profile for the current stream and adjusts connection counts. From 6826e94921b0d7fa49a032d161059c014ce2ddde Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 19:27:45 -0600 Subject: [PATCH 09/58] Add boolean return to Stream.release_stream() Stream.release_stream() now returns True on success and False when no profile info could be found in stream_profile:{stream_id}. This allows callers to detect and handle failed releases. Enhanced logging to include stream ID for better traceability. Based on PR #838 by patchy8736 --- apps/channels/models.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index 9e466cc7..b6cb6ab0 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -207,6 +207,10 @@ class Stream(models.Model): def release_stream(self): """ Called when a stream is finished to release the lock. + + Returns: + bool: True if stream was successfully released, False if + no profile info could be found for cleanup. """ redis_client = RedisClient.get_client() @@ -214,14 +218,17 @@ class Stream(models.Model): # Get the matched profile for cleanup profile_id = redis_client.get(f"stream_profile:{stream_id}") if not profile_id: - logger.debug("Invalid profile ID pulled from stream index") - return + logger.debug( + f"Stream {stream_id}: no profile found in " + f"stream_profile:{stream_id}" + ) + return False redis_client.delete(f"stream_profile:{stream_id}") # Remove profile association profile_id = int(profile_id) logger.debug( - f"Found profile ID {profile_id} associated with stream {stream_id}" + f"Stream {stream_id}: found profile_id={profile_id}" ) profile_connections_key = f"profile_connections:{profile_id}" @@ -231,6 +238,8 @@ class Stream(models.Model): if current_count > 0: redis_client.decr(profile_connections_key) + return True + class ChannelManager(models.Manager): def active(self): From fe17d0d5850c0b58e2cea150d97202e8e713afdb Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 19:49:11 -0600 Subject: [PATCH 10/58] Check release_stream() boolean return at all call sites All 12 call sites for Channel.release_stream() and Stream.release_stream() now check the boolean return value and log warnings when release fails. Files updated: - server.py: 4 call sites (zombie cleanup + _clean_redis_keys) - channel_service.py: 2 call sites (stop_channel) - stream_generator.py: 1 call site (last client disconnect) - views.py: 4 call sites (failed init, redirect, init failure, exception) - url_utils.py: 1 call site (URL generation error) Also fixed bare except clause in _clean_redis_keys and uncaught Stream.DoesNotExist in channel_service.stop_channel. Based on PR #838 by patchy8736 --- apps/proxy/ts_proxy/server.py | 25 +++++++++++++------ .../ts_proxy/services/channel_service.py | 22 ++++++++++------ apps/proxy/ts_proxy/stream_generator.py | 8 +++--- apps/proxy/ts_proxy/url_utils.py | 3 ++- apps/proxy/ts_proxy/views.py | 12 ++++++--- 5 files changed, 47 insertions(+), 23 deletions(-) diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index df51744d..feef4957 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -788,13 +788,17 @@ class ProxyServer: # Force release resources in the Channel model try: channel = Channel.objects.get(uuid=channel_id) - channel.release_stream() - logger.info(f"Released stream allocation for zombie channel {channel_id}") + if not channel.release_stream(): + logger.warning(f"Failed to release stream for zombie channel {channel_id}") + else: + logger.info(f"Released stream allocation for zombie channel {channel_id}") except Exception as e: try: stream = Stream.objects.get(stream_hash=channel_id) - stream.release_stream() - logger.info(f"Released stream allocation for zombie channel {channel_id}") + if not stream.release_stream(): + logger.warning(f"Failed to release stream for zombie channel {channel_id}") + else: + logger.info(f"Released stream allocation for zombie channel {channel_id}") except Exception as e: logger.error(f"Error releasing stream for zombie channel {channel_id}: {e}") @@ -1339,10 +1343,15 @@ class ProxyServer: # Release the channel, stream, and profile keys from the channel try: channel = Channel.objects.get(uuid=channel_id) - channel.release_stream() - except: - stream = Stream.objects.get(stream_hash=channel_id) - stream.release_stream() + if not channel.release_stream(): + logger.debug(f"Channel {channel_id}: release_stream found no keys to clean") + except Channel.DoesNotExist: + try: + stream = Stream.objects.get(stream_hash=channel_id) + if not stream.release_stream(): + logger.debug(f"Stream {channel_id}: release_stream found no keys to clean") + except Stream.DoesNotExist: + logger.debug(f"No Channel or Stream found for {channel_id}") if not self.redis_client: return 0 diff --git a/apps/proxy/ts_proxy/services/channel_service.py b/apps/proxy/ts_proxy/services/channel_service.py index 4c4a73ac..efe07c7e 100644 --- a/apps/proxy/ts_proxy/services/channel_service.py +++ b/apps/proxy/ts_proxy/services/channel_service.py @@ -265,15 +265,23 @@ class ChannelService: # Release the channel in the channel model if applicable try: channel = Channel.objects.get(uuid=channel_id) - channel.release_stream() - logger.info(f"Released channel {channel_id} stream allocation") - model_released = True + model_released = channel.release_stream() + if model_released: + logger.info(f"Released channel {channel_id} stream allocation") + else: + logger.warning(f"Channel {channel_id}: release_stream found no keys to clean") except Channel.DoesNotExist: logger.warning(f"Could not find Channel model for UUID {channel_id}, attempting stream hash") - stream = Stream.objects.get(stream_hash=channel_id) - stream.release_stream() - logger.info(f"Released stream {channel_id} stream allocation") - model_released = True + try: + stream = Stream.objects.get(stream_hash=channel_id) + model_released = stream.release_stream() + if model_released: + logger.info(f"Released stream {channel_id} stream allocation") + else: + logger.warning(f"Stream {channel_id}: release_stream found no keys to clean") + except Stream.DoesNotExist: + logger.error(f"No Channel or Stream found for {channel_id}") + model_released = False except Exception as e: logger.error(f"Error releasing channel stream: {e}") model_released = False diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 771b0aa9..3c2a66fe 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -441,9 +441,11 @@ class StreamGenerator: try: # Get the channel by UUID channel = Channel.objects.get(uuid=self.channel_id) - channel.release_stream() - stream_released = True - logger.debug(f"[{self.client_id}] Released stream for channel {self.channel_id}") + stream_released = channel.release_stream() + if stream_released: + logger.debug(f"[{self.client_id}] Released stream for channel {self.channel_id}") + else: + logger.warning(f"[{self.client_id}] release_stream found no keys for channel {self.channel_id}") except Exception as e: logger.error(f"[{self.client_id}] Error releasing stream for channel {self.channel_id}: {e}") except Exception as e: diff --git a/apps/proxy/ts_proxy/url_utils.py b/apps/proxy/ts_proxy/url_utils.py index 9daaf4ef..fe06ad48 100644 --- a/apps/proxy/ts_proxy/url_utils.py +++ b/apps/proxy/ts_proxy/url_utils.py @@ -162,7 +162,8 @@ def generate_stream_url(channel_id: str) -> Tuple[str, str, bool, Optional[int]] return stream_url, stream_user_agent, transcode, stream_profile_id except Exception as e: logger.error(f"Error generating stream URL for channel {channel_id}: {e}") - channel.release_stream() + if not channel.release_stream(): + logger.warning(f"Failed to release stream for channel {channel_id} after URL generation error") return None, None, False, None except Exception as e: logger.error(f"Error generating stream URL: {e}") diff --git a/apps/proxy/ts_proxy/views.py b/apps/proxy/ts_proxy/views.py index 6980f957..5060a3a7 100644 --- a/apps/proxy/ts_proxy/views.py +++ b/apps/proxy/ts_proxy/views.py @@ -222,7 +222,8 @@ def stream_ts(request, channel_id): if stream_url is None: # Release any connection slot that may have been allocated # by the error-checking get_stream() call during retries - channel.release_stream() + if not channel.release_stream(): + logger.debug(f"[{client_id}] release_stream found no keys during failed init cleanup") # Get the specific error message if available wait_duration = f"{int(time.time() - wait_start_time)}s" @@ -314,7 +315,8 @@ def stream_ts(request, channel_id): f"[{client_id}] Alternate stream #{alt['stream_id']} failed validation: {message}" ) # Release stream lock before redirecting - channel.release_stream() + if not channel.release_stream(): + logger.warning(f"[{client_id}] Failed to release stream before redirect") # Final decision based on validation results if is_valid: logger.info( @@ -351,7 +353,8 @@ def stream_ts(request, channel_id): if not success: if connection_allocated: - channel.release_stream() + if not channel.release_stream(): + logger.warning(f"[{client_id}] Failed to release stream after init failure") connection_allocated = False return JsonResponse( {"error": "Failed to initialize channel"}, status=500 @@ -521,7 +524,8 @@ def stream_ts(request, channel_id): logger.error(f"Error in stream_ts: {e}", exc_info=True) if connection_allocated: try: - channel.release_stream() + if not channel.release_stream(): + logger.warning(f"[{client_id}] Failed to release stream in exception handler") except Exception: pass return JsonResponse({"error": str(e)}, status=500) From 6360271e2830831fbfca9bf5c938e29dd25d7ef5 Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 20:09:17 -0600 Subject: [PATCH 11/58] Fix unprotected _clean_redis_keys() call in check_if_channel_exists Wrap _clean_redis_keys() call in check_if_channel_exists() with try/except to prevent exception propagation when Channel/Stream lookup or release_stream() encounters DB or Redis errors. --- apps/proxy/ts_proxy/server.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index feef4957..3a6e8ae3 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -766,7 +766,10 @@ class ProxyServer: if self.redis_client.exists(key): # Found orphaned keys without metadata - clean them up logger.warning(f"Found orphaned keys for channel {channel_id} without metadata - cleaning up") - self._clean_redis_keys(channel_id) + try: + self._clean_redis_keys(channel_id) + except Exception as e: + logger.error(f"Error cleaning redis keys for channel {channel_id}: {e}") return False return False From 8bb8a0fca904ccfc3c554ac6702e88ab94d3f094 Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 20:52:30 -0600 Subject: [PATCH 12/58] fix(ts-proxy): Remove redundant local import causing UnboundLocalError The local `from apps.channels.models import Channel` inside a conditional block in _handle_client_disconnect() shadowed the top-level import. When the condition was False, Python's function-level scoping caused line 462's Channel.objects.get() to raise UnboundLocalError. Channel is already imported at module level (line 11), making the local import unnecessary. Discovered when testing proxy changes --- apps/proxy/ts_proxy/stream_generator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 3c2a66fe..875d9d0e 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -437,7 +437,6 @@ class StreamGenerator: client_count = proxy_server.client_managers[self.channel_id].get_total_client_count() # Only the last client or owner should release the stream if client_count <= 1 and proxy_server.am_i_owner(self.channel_id): - from apps.channels.models import Channel try: # Get the channel by UUID channel = Channel.objects.get(uuid=self.channel_id) From 837ba847685bb93f6d9351720e68c3ca19c6d04e Mon Sep 17 00:00:00 2001 From: None Date: Wed, 11 Feb 2026 23:57:10 -0600 Subject: [PATCH 13/58] fix(ts-proxy): Restore Stream.get_stream() with atomic slot reservation Stream.get_stream() was incorrectly removed as dead code in commit 33f68a98. It is actually called get_stream_object() in views.py when a stream hash (not channel UUID) is used, such as during stream preview. The variable is named channel but holds a Stream instance, which masked the dependency during analysis. Restored with the INCR-first-then-check pattern applied, fixing the same TOCTOU vulnerability the original method had. --- apps/channels/models.py | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/apps/channels/models.py b/apps/channels/models.py index b6cb6ab0..b9c1dde1 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -204,6 +204,52 @@ class Stream(models.Model): return stream_profile + def get_stream(self): + """ + Finds an available profile for this stream and reserves a connection slot. + + Returns: + Tuple[Optional[int], Optional[int], Optional[str]]: (stream_id, profile_id, error_reason) + """ + redis_client = RedisClient.get_client() + profile_id = redis_client.get(f"stream_profile:{self.id}") + if profile_id: + profile_id = int(profile_id) + return self.id, profile_id, None + + # Retrieve the M3U account associated with the stream. + m3u_account = self.m3u_account + m3u_profiles = m3u_account.profiles.all() + default_profile = next((obj for obj in m3u_profiles if obj.is_default), None) + profiles = [default_profile] + [ + obj for obj in m3u_profiles if not obj.is_default + ] + + for profile in profiles: + logger.info(profile) + # Skip inactive profiles + if profile.is_active == False: + continue + + # Atomic slot reservation: INCR first, check, rollback if over capacity + if profile.max_streams == 0: + reserved = True + else: + profile_connections_key = f"profile_connections:{profile.id}" + new_count = redis_client.incr(profile_connections_key) + if new_count <= profile.max_streams: + reserved = True + else: + redis_client.decr(profile_connections_key) + reserved = False + + if reserved: + redis_client.set(f"channel_stream:{self.id}", self.id) + redis_client.set(f"stream_profile:{self.id}", profile.id) + return self.id, profile.id, None + + return None, None, "All active M3U profiles have reached maximum connection limits" + def release_stream(self): """ Called when a stream is finished to release the lock. From 20ce45f7adfd7a0f959207f09125accec2e93806 Mon Sep 17 00:00:00 2001 From: None Date: Thu, 12 Feb 2026 00:02:40 -0600 Subject: [PATCH 14/58] fix(ts-proxy): Add Stream fallback in cleanup path to prevent counter leak on stream preview When previewing a stream directly (via stream hash), the cleanup in stream_generator.py tried Channel.objects.get(uuid=stream_hash) which failed with ValidationError since the hash is not a UUID. This prevented release_stream() from running, permanently leaking the connection counter. With max_streams=1, a single preview permanently blocked all subsequent streams. Added Stream.objects.get(stream_hash=...) fallback so the cleanup path works for both Channel (UUID) and Stream (hash) identifiers. Also added Stream import to stream_generator.py. --- apps/proxy/ts_proxy/stream_generator.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 875d9d0e..f43eb528 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -8,7 +8,7 @@ import logging import threading import gevent # Add this import at the top of your file from apps.proxy.config import TSConfig as Config -from apps.channels.models import Channel +from apps.channels.models import Channel, Stream from core.utils import log_system_event from .server import ProxyServer from .utils import create_ts_packet, get_logger @@ -438,9 +438,12 @@ class StreamGenerator: # Only the last client or owner should release the stream if client_count <= 1 and proxy_server.am_i_owner(self.channel_id): try: - # Get the channel by UUID - channel = Channel.objects.get(uuid=self.channel_id) - stream_released = channel.release_stream() + # Try Channel first (normal flow), fall back to Stream (preview flow) + try: + obj = Channel.objects.get(uuid=self.channel_id) + except (Channel.DoesNotExist, Exception): + obj = Stream.objects.get(stream_hash=self.channel_id) + stream_released = obj.release_stream() if stream_released: logger.debug(f"[{self.client_id}] Released stream for channel {self.channel_id}") else: From 51b58247378a63de9adfa0dd075adb85f8cb5e44 Mon Sep 17 00:00:00 2001 From: None Date: Thu, 12 Feb 2026 00:09:29 -0600 Subject: [PATCH 15/58] Fix cleanup paths for stream preview Fix exception handling in _clean_redis_keys() and stop_channel() where `except Channel.DoesNotExist` didn't catch ValidationError from invalid UUID format, bypassing the Stream fallback path. --- apps/proxy/ts_proxy/server.py | 4 ++-- apps/proxy/ts_proxy/services/channel_service.py | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index 3a6e8ae3..d036bc2e 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -1348,12 +1348,12 @@ class ProxyServer: channel = Channel.objects.get(uuid=channel_id) if not channel.release_stream(): logger.debug(f"Channel {channel_id}: release_stream found no keys to clean") - except Channel.DoesNotExist: + except (Channel.DoesNotExist, Exception): try: stream = Stream.objects.get(stream_hash=channel_id) if not stream.release_stream(): logger.debug(f"Stream {channel_id}: release_stream found no keys to clean") - except Stream.DoesNotExist: + except (Stream.DoesNotExist, Exception): logger.debug(f"No Channel or Stream found for {channel_id}") if not self.redis_client: diff --git a/apps/proxy/ts_proxy/services/channel_service.py b/apps/proxy/ts_proxy/services/channel_service.py index efe07c7e..e765ece3 100644 --- a/apps/proxy/ts_proxy/services/channel_service.py +++ b/apps/proxy/ts_proxy/services/channel_service.py @@ -270,7 +270,7 @@ class ChannelService: logger.info(f"Released channel {channel_id} stream allocation") else: logger.warning(f"Channel {channel_id}: release_stream found no keys to clean") - except Channel.DoesNotExist: + except (Channel.DoesNotExist, Exception): logger.warning(f"Could not find Channel model for UUID {channel_id}, attempting stream hash") try: stream = Stream.objects.get(stream_hash=channel_id) @@ -279,12 +279,9 @@ class ChannelService: logger.info(f"Released stream {channel_id} stream allocation") else: logger.warning(f"Stream {channel_id}: release_stream found no keys to clean") - except Stream.DoesNotExist: - logger.error(f"No Channel or Stream found for {channel_id}") + except (Stream.DoesNotExist, Exception) as e: + logger.error(f"No Channel or Stream found for {channel_id}: {e}") model_released = False - except Exception as e: - logger.error(f"Error releasing channel stream: {e}") - model_released = False return { 'status': 'success', From c69718b55987362403d468913dbe6cd7dd040954 Mon Sep 17 00:00:00 2001 From: None Date: Tue, 17 Feb 2026 21:18:20 -0600 Subject: [PATCH 16/58] Fix: Make release_stream() idempotent to prevent double-DECR on rapid channel switching Multiple code paths call release_stream() during channel shutdown (stream generator cleanup, _clean_redis_keys, ChannelService.stop_channel), causing the profile_connections counter to be decremented multiple times for the same stream. The metadata hash fallback persisted longer than the primary keys, allowing subsequent calls to find stale stream_id/m3u_profile and DECR again. Changes: - Clear STREAM_ID and M3U_PROFILE metadata fields after decrementing in both the primary and fallback paths, making release_stream() safe to call multiple times - Add metadata fallback when stream_profile:{id} key is missing but channel_stream:{id} exists, preventing a counter leak in that edge case - Replace redundant get_stream() call in views.py with direct Redis reads to avoid side-effect INCR from a method that can allocate connection slots --- apps/channels/models.py | 47 +++++++++++++++++++++++++++++------- apps/proxy/ts_proxy/views.py | 14 +++++++++-- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/apps/channels/models.py b/apps/channels/models.py index b9c1dde1..217a92c0 100644 --- a/apps/channels/models.py +++ b/apps/channels/models.py @@ -564,6 +564,14 @@ class Channel(models.Model): redis_client.delete(f"channel_stream:{self.id}") redis_client.delete(f"stream_profile:{stream_id}") + # Clear metadata fields so duplicate release_stream() calls + # won't find them and DECR again + redis_client.hdel( + metadata_key, + ChannelMetadataField.STREAM_ID, + ChannelMetadataField.M3U_PROFILE, + ) + profile_connections_key = f"profile_connections:{profile_id}" current_count = int( redis_client.get(profile_connections_key) or 0 @@ -588,16 +596,27 @@ class Channel(models.Model): # Get the matched profile for cleanup profile_id = redis_client.get(f"stream_profile:{stream_id}") - if not profile_id: - logger.debug( - f"Channel {self.uuid}: no profile found for " - f"stream_profile:{stream_id}" + if profile_id: + redis_client.delete(f"stream_profile:{stream_id}") # Remove profile association + profile_id = int(profile_id) + else: + # stream_profile key missing — try metadata hash fallback + metadata_key = RedisKeys.channel_metadata(str(self.uuid)) + meta_profile_id = redis_client.hget( + metadata_key, ChannelMetadataField.M3U_PROFILE ) - return False - - redis_client.delete(f"stream_profile:{stream_id}") # Remove profile association - - profile_id = int(profile_id) + if meta_profile_id: + profile_id = int(meta_profile_id) + logger.debug( + f"Channel {self.uuid}: recovered profile_id={profile_id} " + f"from metadata fallback (stream_profile:{stream_id} was missing)" + ) + else: + logger.warning( + f"Channel {self.uuid}: no profile found for " + f"stream_profile:{stream_id} or in metadata fallback" + ) + return False logger.debug( f"Channel {self.uuid}: found profile_id={profile_id} for " f"stream {stream_id}" @@ -610,6 +629,16 @@ class Channel(models.Model): if current_count > 0: redis_client.decr(profile_connections_key) + # Clear metadata fields so duplicate release_stream() calls + # (e.g. from _clean_redis_keys or ChannelService.stop_channel) + # won't find them via fallback and DECR again + metadata_key = RedisKeys.channel_metadata(str(self.uuid)) + redis_client.hdel( + metadata_key, + ChannelMetadataField.STREAM_ID, + ChannelMetadataField.M3U_PROFILE, + ) + return True def update_stream_profile(self, new_profile_id): diff --git a/apps/proxy/ts_proxy/views.py b/apps/proxy/ts_proxy/views.py index 5060a3a7..90ac2673 100644 --- a/apps/proxy/ts_proxy/views.py +++ b/apps/proxy/ts_proxy/views.py @@ -244,8 +244,18 @@ def stream_ts(request, channel_id): if needs_initialization: connection_allocated = True - # Get the stream ID from the channel - stream_id, m3u_profile_id, _ = channel.get_stream() + # Read stream assignment from Redis (already set by generate_stream_url → get_stream). + # Avoid calling get_stream() again — (INCR profile counter) + # It could double-allocate if the keys were cleared by a concurrent release. + stream_id = None + m3u_profile_id = None + if proxy_server.redis_client: + stream_id_bytes = proxy_server.redis_client.get(f"channel_stream:{channel.id}") + if stream_id_bytes: + stream_id = int(stream_id_bytes) + profile_id_bytes = proxy_server.redis_client.get(f"stream_profile:{stream_id}") + if profile_id_bytes: + m3u_profile_id = int(profile_id_bytes) logger.info( f"Channel {channel_id} using stream ID {stream_id}, m3u account profile ID {m3u_profile_id}" ) From 9b6c793b1186da86c2860f74006ee8b42d23ad1c Mon Sep 17 00:00:00 2001 From: Endoze Date: Thu, 26 Feb 2026 09:41:57 -0500 Subject: [PATCH 17/58] fix(wsgi): ensure gevent monkey-patching before Django import chain Celery's Redis connection pool was silently failing in modular deployment mode because Django's setup imports celery.py before uWSGI's gevent plugin reliably patches stdlib sockets. Moving patch_all() to the top of wsgi.py guarantees all broker connections are gevent-aware regardless of uWSGI initialization order. --- dispatcharr/tests/__init__.py | 0 dispatcharr/tests/test_wsgi.py | 39 ++++++++++++++++++++++++++++++++++ dispatcharr/wsgi.py | 15 +++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 dispatcharr/tests/__init__.py create mode 100644 dispatcharr/tests/test_wsgi.py diff --git a/dispatcharr/tests/__init__.py b/dispatcharr/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/dispatcharr/tests/test_wsgi.py b/dispatcharr/tests/test_wsgi.py new file mode 100644 index 00000000..cdf7a2e9 --- /dev/null +++ b/dispatcharr/tests/test_wsgi.py @@ -0,0 +1,39 @@ +"""Tests for dispatcharr/wsgi.py gevent monkey-patching.""" + +import os +import subprocess +import sys +import unittest + + +class TestWSGIGeventPatching(unittest.TestCase): + """Verify that wsgi.py applies gevent monkey-patching before Django imports. + + These tests run in subprocesses because monkey.patch_all() must execute + before any other imports. By the time Django's test runner loads this + file, ssl/socket are already imported, so calling patch_all() in-process + would fail. A subprocess mirrors how uWSGI actually loads wsgi.py. + """ + + def test_socket_is_patched_after_wsgi_import(self): + """Loading wsgi.py first (as uWSGI does) should patch sockets.""" + result = subprocess.run( + [ + sys.executable, "-c", + "import dispatcharr.wsgi; " + "from gevent import monkey; " + "assert monkey.is_module_patched('socket'), " + "'socket module was not patched by wsgi.py'; " + "print('PASS')", + ], + capture_output=True, + text=True, + env={**os.environ, "DJANGO_SETTINGS_MODULE": "dispatcharr.settings"}, + timeout=60, + cwd="/app", + ) + self.assertEqual( + result.returncode, + 0, + f"monkey-patching check failed:\n{result.stderr}", + ) diff --git a/dispatcharr/wsgi.py b/dispatcharr/wsgi.py index 29c2ba01..6e8860af 100644 --- a/dispatcharr/wsgi.py +++ b/dispatcharr/wsgi.py @@ -1,6 +1,21 @@ """ WSGI config for dispatcharr project. """ +# When running under uWSGI with gevent, ensure monkey-patching is fully +# applied before any other imports. Django's setup triggers the import of +# dispatcharr/__init__.py → celery.py, which initialises the Celery broker +# transport. If those imports happen before sockets are patched, Celery's +# Redis connection pool holds unpatched sockets that silently fail under +# gevent's cooperative scheduling. Calling patch_all() here—before the +# Django import chain—guarantees every socket (including pooled broker +# connections) is gevent-aware. The call is idempotent, so it's harmless +# if uWSGI's gevent plugin has already patched. +try: + from gevent import monkey + monkey.patch_all() +except ImportError: + pass + import os from django.core.wsgi import get_wsgi_application From 8bd38ad71c455317d9a0753887b223bd3321fee5 Mon Sep 17 00:00:00 2001 From: PFalko Date: Fri, 27 Feb 2026 16:40:28 +0100 Subject: [PATCH 18/58] Fix stream ownership bugs causing streams to die after 30-200s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three interrelated bugs cause TS proxy streams to terminate prematurely in multi-worker uWSGI deployments: 1. Double ProxyServer instantiation: ProxyConfig.ready() in apps/proxy/apps.py calls TSProxyServer() directly, bypassing get_instance(). The subsequent TSProxyConfig.ready() call to get_instance() creates a second instance. Each instance starts its own cleanup thread, but only one holds channel data — the orphaned cleanup thread cannot extend ownership. 2. Redis flushdb() on every client init: RedisClient.get_client() in core/utils.py calls flushdb() whenever a new connection is created. Celery autoscale workers spawning mid-stream nuke all Redis keys including ownership, client records, and channel metadata. 3. No recovery from expired ownership: get_channel_owner() has a TOCTOU bug (two separate GET calls in a lambda). extend_ownership() silently fails when keys expire. Non-owner cleanup unconditionally kills streams even when the worker holds the stream_manager. Fixes: - Use TSProxyServer.get_instance() in ProxyConfig.ready() - Remove flushdb() from Redis client initialization - Use sentinel pattern for gevent-safe singleton (threading.Lock does not work with gevent greenlets) - Single GET in get_channel_owner() to avoid TOCTOU race - Re-acquire expired ownership keys in extend_ownership() - Attempt re-acquisition before cleanup in non-owner path Relates to #992, #980 --- apps/proxy/apps.py | 4 +-- apps/proxy/ts_proxy/server.py | 66 +++++++++++++++++++++++++++++------ core/utils.py | 1 - 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/apps/proxy/apps.py b/apps/proxy/apps.py index d1c8b966..b33d46cc 100644 --- a/apps/proxy/apps.py +++ b/apps/proxy/apps.py @@ -12,6 +12,6 @@ class ProxyConfig(AppConfig): from .hls_proxy.server import ProxyServer as HLSProxyServer from .ts_proxy.server import ProxyServer as TSProxyServer - # Initialize proxy servers + # Initialize proxy servers (TS uses singleton to prevent duplicate instances) self.hls_proxy = HLSProxyServer() - self.ts_proxy = TSProxyServer() + self.ts_proxy = TSProxyServer.get_instance() diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index df51744d..b28154b3 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -34,18 +34,29 @@ logger = get_logger() class ProxyServer: """Manages TS proxy server instance with worker coordination""" _instance = None + _INITIALIZING = object() # sentinel for gevent-safe singleton @classmethod def get_instance(cls): - if cls._instance is None: + inst = cls._instance + if inst is not None and inst is not cls._INITIALIZING: + return inst + if inst is None: + cls._instance = cls._INITIALIZING from .server import ProxyServer from .stream_manager import StreamManager from .stream_buffer import StreamBuffer from .client_manager import ClientManager - - cls._instance = ProxyServer() - - return cls._instance + real_instance = ProxyServer() + cls._instance = real_instance + return real_instance + # Another greenlet is initializing — wait for completion + import time + while True: + inst = cls._instance + if inst is not None and inst is not cls._INITIALIZING: + return inst + time.sleep(0.05) def __init__(self): """Initialize proxy server with worker identification""" @@ -353,9 +364,16 @@ class ProxyServer: try: lock_key = RedisKeys.channel_owner(channel_id) - return self._execute_redis_command( - lambda: self.redis_client.get(lock_key).decode('utf-8') if self.redis_client.get(lock_key) else None + result = self._execute_redis_command( + lambda: self.redis_client.get(lock_key) ) + if result is None: + return None + try: + return result.decode('utf-8') + except (AttributeError, UnicodeDecodeError) as e: + logger.error(f"Error decoding channel owner for {channel_id}: {e}, raw={result!r}") + return None except Exception as e: logger.error(f"Error getting channel owner: {e}") return None @@ -433,7 +451,7 @@ class ProxyServer: logger.error(f"Error releasing channel ownership: {e}") def extend_ownership(self, channel_id, ttl=30): - """Extend ownership lease with grace period""" + """Extend ownership lease, re-acquiring if key expired""" if not self.redis_client: return False @@ -441,10 +459,24 @@ class ProxyServer: lock_key = RedisKeys.channel_owner(channel_id) current = self.redis_client.get(lock_key) - # Only extend if we're still the owner - if current and current.decode('utf-8') == self.worker_id: + if current is None: + # Key expired — re-acquire if we have the stream_manager + if channel_id in self.stream_managers: + acquired = self.redis_client.setnx(lock_key, self.worker_id) + if acquired: + self.redis_client.expire(lock_key, ttl) + logger.warning(f"Re-acquired expired ownership for channel {channel_id}") + return True + else: + new_owner = self.redis_client.get(lock_key) + logger.warning(f"Could not re-acquire ownership for {channel_id}, new owner: {new_owner}") + return False + return False + + if current.decode('utf-8') == self.worker_id: self.redis_client.expire(lock_key, ttl) return True + return False except Exception as e: logger.error(f"Error extending ownership: {e}") @@ -1173,6 +1205,20 @@ class ProxyServer: else: # === NON-OWNER CHANNEL HANDLING === + # Safety: if we have a stream_manager, we ARE the real owner + # but the Redis key may have expired. Try to re-acquire. + if channel_id in self.stream_managers: + logger.warning( + f"Ownership gap for {channel_id}: this worker has stream_manager " + f"but am_i_owner returned False. Attempting re-acquisition." + ) + reacquired = self.extend_ownership(channel_id) + if reacquired: + logger.info(f"Successfully re-acquired ownership for {channel_id}") + continue + else: + logger.error(f"Failed to re-acquire ownership for {channel_id}, will clean up") + # For channels we don't own, check if they've been stopped/cleaned up in Redis if self.redis_client: # Method 1: Check for stopping key diff --git a/core/utils.py b/core/utils.py index a5ea41df..f0e02bb4 100644 --- a/core/utils.py +++ b/core/utils.py @@ -81,7 +81,6 @@ class RedisClient: # Validate connection with ping client.ping() - client.flushdb() # Disable persistence on first connection - improves performance # Only try to disable if not in a read-only environment From a318d919d8263110fbd088222edc6f69ce1d6b17 Mon Sep 17 00:00:00 2001 From: Jonathan Caicedo Date: Thu, 26 Feb 2026 09:19:19 -0500 Subject: [PATCH 19/58] feat: filter channels that contain stale streams closes: #1025 --- apps/channels/api_views.py | 4 +++ .../src/components/tables/ChannelsTable.jsx | 24 +++++++++++++---- .../ChannelsTable/ChannelTableHeader.jsx | 27 ++++++++++++++++++- frontend/src/components/tables/table.css | 2 +- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/channels/api_views.py b/apps/channels/api_views.py index d0fb3d97..b77cf60d 100644 --- a/apps/channels/api_views.py +++ b/apps/channels/api_views.py @@ -571,6 +571,7 @@ class ChannelViewSet(viewsets.ModelViewSet): channel_profile_id = self.request.query_params.get("channel_profile_id") show_disabled_param = self.request.query_params.get("show_disabled", None) only_streamless = self.request.query_params.get("only_streamless", None) + only_stale = self.request.query_params.get("only_stale", None) if channel_profile_id: try: @@ -590,6 +591,9 @@ class ChannelViewSet(viewsets.ModelViewSet): if only_streamless: q_filters &= Q(streams__isnull=True) + if only_stale: + # Filter channels that have at least one related stream marked as stale + q_filters &= Q(streams__is_stale=True) if self.request.user.user_level < 10: filters["user_level__lte"] = self.request.user.user_level diff --git a/frontend/src/components/tables/ChannelsTable.jsx b/frontend/src/components/tables/ChannelsTable.jsx index 06fe652a..75bf0e33 100644 --- a/frontend/src/components/tables/ChannelsTable.jsx +++ b/frontend/src/components/tables/ChannelsTable.jsx @@ -321,6 +321,7 @@ const ChannelsTable = ({ onReady }) => { const [showDisabled, setShowDisabled] = useState(true); const [showOnlyStreamlessChannels, setShowOnlyStreamlessChannels] = useState(false); + const [showOnlyStaleChannels, setShowOnlyStaleChannels] = useState(false); const [paginationString, setPaginationString] = useState(''); const [filters, setFilters] = useState({ @@ -424,6 +425,9 @@ const ChannelsTable = ({ onReady }) => { if (showOnlyStreamlessChannels === true) { params.append('only_streamless', true); } + if (showOnlyStaleChannels === true) { + params.append('only_stale', true); + } // Apply sorting if (sorting.length > 0) { @@ -511,6 +515,7 @@ const ChannelsTable = ({ onReady }) => { showDisabled, selectedProfileId, showOnlyStreamlessChannels, + showOnlyStaleChannels, ]); const stopPropagation = useCallback((e) => { @@ -1128,11 +1133,18 @@ const ChannelsTable = ({ onReady }) => { getRowStyles: (row) => { const hasStreams = row.original.streams && row.original.streams.length > 0; - return hasStreams - ? {} // Default style for channels with streams - : { - className: 'no-streams-row', // Add a class instead of background color - }; + const hasStaleStreams = + row.original.streams && row.original.streams.some((s) => s.is_stale); + + if (!hasStreams) { + return { className: 'no-streams-row' }; + } + + if (hasStaleStreams) { + return { className: 'stale-streams-row' }; + } + + return {}; }, }); @@ -1435,6 +1447,8 @@ const ChannelsTable = ({ onReady }) => { setShowDisabled={setShowDisabled} showOnlyStreamlessChannels={showOnlyStreamlessChannels} setShowOnlyStreamlessChannels={setShowOnlyStreamlessChannels} + showOnlyStaleChannels={showOnlyStaleChannels} + setShowOnlyStaleChannels={setShowOnlyStaleChannels} /> {/* Table or ghost empty state inside Paper */} diff --git a/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx b/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx index ac2f4d8d..c8023283 100644 --- a/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx +++ b/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx @@ -116,6 +116,8 @@ const ChannelTableHeader = ({ setShowDisabled, showOnlyStreamlessChannels, setShowOnlyStreamlessChannels, + showOnlyStaleChannels, + setShowOnlyStaleChannels, }) => { const theme = useMantineTheme(); @@ -218,7 +220,21 @@ const ChannelTableHeader = ({ }; const toggleShowOnlyStreamlessChannels = () => { - setShowOnlyStreamlessChannels(!showOnlyStreamlessChannels); + const newVal = !showOnlyStreamlessChannels; + setShowOnlyStreamlessChannels(newVal); + if (newVal) { + // Ensure stale toggle is cleared when enabling streamless-only + setShowOnlyStaleChannels(false); + } + }; + + const toggleShowOnlyStaleChannels = () => { + const newVal = !showOnlyStaleChannels; + setShowOnlyStaleChannels(newVal); + if (newVal) { + // Ensure streamless toggle is cleared when enabling stale-only + setShowOnlyStreamlessChannels(false); + } }; const toggleHeaderPinned = () => { @@ -307,6 +323,15 @@ const ChannelTableHeader = ({ > Only Empty Channels + + : + } + > + Only Containing Stale Streams + diff --git a/frontend/src/components/tables/table.css b/frontend/src/components/tables/table.css index b957d741..688cf486 100644 --- a/frontend/src/components/tables/table.css +++ b/frontend/src/components/tables/table.css @@ -92,7 +92,7 @@ html { opacity: 0; } - *:hover > .resizer { + *:hover>.resizer { opacity: 1; } } From 31fb27dffc279b2cc34875c16b313b8a4c0424b5 Mon Sep 17 00:00:00 2001 From: Jonathan Caicedo Date: Thu, 26 Feb 2026 11:26:39 -0500 Subject: [PATCH 20/58] perf: avoid double-calling row.original.streams - reuse hasStreams This is a micro-optimization, but prevents checking row.original.streams twice and also guards checking for stale streams unless we know we have streams. The performance hit before would have been tiny, but every bit counts. --- frontend/src/components/tables/ChannelsTable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/tables/ChannelsTable.jsx b/frontend/src/components/tables/ChannelsTable.jsx index 75bf0e33..25bea550 100644 --- a/frontend/src/components/tables/ChannelsTable.jsx +++ b/frontend/src/components/tables/ChannelsTable.jsx @@ -1134,7 +1134,7 @@ const ChannelsTable = ({ onReady }) => { const hasStreams = row.original.streams && row.original.streams.length > 0; const hasStaleStreams = - row.original.streams && row.original.streams.some((s) => s.is_stale); + hasStreams && row.original.streams.some((s) => s.is_stale); if (!hasStreams) { return { className: 'no-streams-row' }; From 4dfe172fc4dfdea911ea2abee2ba0897963b3fd8 Mon Sep 17 00:00:00 2001 From: Jonathan Caicedo Date: Sat, 28 Feb 2026 12:20:48 -0500 Subject: [PATCH 21/58] tweak: label name + CSS class name --- frontend/src/components/tables/ChannelsTable.jsx | 2 +- .../tables/ChannelsTable/ChannelTableHeader.jsx | 2 +- frontend/src/components/tables/table.css | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/tables/ChannelsTable.jsx b/frontend/src/components/tables/ChannelsTable.jsx index 25bea550..833bdb4f 100644 --- a/frontend/src/components/tables/ChannelsTable.jsx +++ b/frontend/src/components/tables/ChannelsTable.jsx @@ -1141,7 +1141,7 @@ const ChannelsTable = ({ onReady }) => { } if (hasStaleStreams) { - return { className: 'stale-streams-row' }; + return { className: 'partially-stale-streams-row' }; } return {}; diff --git a/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx b/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx index c8023283..cda9b496 100644 --- a/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx +++ b/frontend/src/components/tables/ChannelsTable/ChannelTableHeader.jsx @@ -330,7 +330,7 @@ const ChannelTableHeader = ({ showOnlyStaleChannels ? : } > - Only Containing Stale Streams + Has Stale Streams diff --git a/frontend/src/components/tables/table.css b/frontend/src/components/tables/table.css index 688cf486..0e02dc1e 100644 --- a/frontend/src/components/tables/table.css +++ b/frontend/src/components/tables/table.css @@ -140,6 +140,17 @@ html { background-color: color-mix(in srgb, rgb(239, 68, 68) 30%, #27272a) !important; } +/* Rows that contain some stale streams (may be mixed with active) */ +.partially-stale-streams-row { + background-color: rgba(239, 154, 68, 0.15) !important; + /* orange-500 @ 12% */ + box-shadow: inset 0 0 0 1px rgba(239, 154, 68, 0.3); +} + +.table-striped .tbody .tr.partially-stale-streams-row:hover { + background-color: rgba(239, 154, 68, 0.3) !important; +} + /* Prevent text selection when shift key is pressed */ .shift-key-active { cursor: pointer !important; From 7fed49a3340e70e739ee7d72954be1ea6629d7cb Mon Sep 17 00:00:00 2001 From: Claude Code Date: Mon, 2 Mar 2026 13:23:45 +0000 Subject: [PATCH 22/58] Address review feedback from CodeBormen - Use atomic SET NX EX instead of separate SETNX + EXPIRE to prevent zombie locks if the process crashes between the two calls - Replace time.sleep() with gevent.sleep() in get_instance() spin-wait to avoid blocking the greenlet hub - Defer cleanup when ownership is lost but clients are still connected, giving the new owner time to establish its stream before we tear down Co-Authored-By: Claude Opus 4.6 --- apps/proxy/ts_proxy/server.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index b28154b3..b0b1a08e 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -51,12 +51,11 @@ class ProxyServer: cls._instance = real_instance return real_instance # Another greenlet is initializing — wait for completion - import time while True: inst = cls._instance if inst is not None and inst is not cls._INITIALIZING: return inst - time.sleep(0.05) + gevent.sleep(0.05) def __init__(self): """Initialize proxy server with worker identification""" @@ -462,9 +461,8 @@ class ProxyServer: if current is None: # Key expired — re-acquire if we have the stream_manager if channel_id in self.stream_managers: - acquired = self.redis_client.setnx(lock_key, self.worker_id) + acquired = self.redis_client.set(lock_key, self.worker_id, nx=True, ex=ttl) if acquired: - self.redis_client.expire(lock_key, ttl) logger.warning(f"Re-acquired expired ownership for channel {channel_id}") return True else: @@ -1217,6 +1215,19 @@ class ProxyServer: logger.info(f"Successfully re-acquired ownership for {channel_id}") continue else: + # Defer cleanup if we still have active clients — give the + # new owner time to spin up its own stream before we tear + # ours down, so viewers don't get disconnected. + has_clients = ( + channel_id in self.client_managers + and self.client_managers[channel_id].get_client_count() > 0 + ) + if has_clients: + logger.warning( + f"Ownership lost for {channel_id} but {self.client_managers[channel_id].get_client_count()} " + f"client(s) still connected — deferring cleanup to next cycle" + ) + continue logger.error(f"Failed to re-acquire ownership for {channel_id}, will clean up") # For channels we don't own, check if they've been stopped/cleaned up in Redis From 1e629d57ecc8c9d2cadd4e0ccf163b28ab9de0ef Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 10:11:45 -0600 Subject: [PATCH 23/58] Refactor ChannelsTable row styling logic to use memoized class mapping for improved performance and readability. --- .../src/components/tables/ChannelsTable.jsx | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/tables/ChannelsTable.jsx b/frontend/src/components/tables/ChannelsTable.jsx index 9e7782be..c627495a 100644 --- a/frontend/src/components/tables/ChannelsTable.jsx +++ b/frontend/src/components/tables/ChannelsTable.jsx @@ -269,6 +269,19 @@ const ChannelsTable = ({ onReady }) => { // store/channelsTable const data = useChannelsTableStore((s) => s.channels); const pageCount = useChannelsTableStore((s) => s.pageCount); + + const rowClassMap = useMemo(() => { + const map = {}; + for (const channel of data) { + const hasStreams = channel.streams?.length > 0; + if (!hasStreams) { + map[channel.id] = 'no-streams-row'; + } else if (channel.streams.some((s) => s.is_stale)) { + map[channel.id] = 'partially-stale-streams-row'; + } + } + return map; + }, [data]); const setSelectedChannelIds = useChannelsTableStore( (s) => s.setSelectedChannelIds ); @@ -1130,20 +1143,8 @@ const ChannelsTable = ({ onReady }) => { epg: renderHeaderCell, }, getRowStyles: (row) => { - const hasStreams = - row.original.streams && row.original.streams.length > 0; - const hasStaleStreams = - hasStreams && row.original.streams.some((s) => s.is_stale); - - if (!hasStreams) { - return { className: 'no-streams-row' }; - } - - if (hasStaleStreams) { - return { className: 'partially-stale-streams-row' }; - } - - return {}; + const cls = rowClassMap[row.original.id]; + return cls ? { className: cls } : {}; }, }); From eb020724311797c4d4f821ea5c759d921424415a Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 10:20:37 -0600 Subject: [PATCH 24/58] Renamed "partially-stale-streams-row" to "has-stale-streams-row" for accuracy. Also updated the changelog. --- CHANGELOG.md | 4 ++++ frontend/src/components/tables/ChannelsTable.jsx | 2 +- frontend/src/components/tables/table.css | 6 +++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af516928..c6606c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Channel table filter for channels that have stale streams: A new "Has Stale Streams" filter option in the channel table header menu highlights and filters channels containing at least one stale stream. Channels with stale streams are visually distinguished with an orange tint. The filter is mutually exclusive with "Only Empty Channels". - Thanks [@JCBird1012](https://github.com/JCBird1012) + ## [0.20.2] - 2026-03-03 ### Security diff --git a/frontend/src/components/tables/ChannelsTable.jsx b/frontend/src/components/tables/ChannelsTable.jsx index c627495a..b6b93f43 100644 --- a/frontend/src/components/tables/ChannelsTable.jsx +++ b/frontend/src/components/tables/ChannelsTable.jsx @@ -277,7 +277,7 @@ const ChannelsTable = ({ onReady }) => { if (!hasStreams) { map[channel.id] = 'no-streams-row'; } else if (channel.streams.some((s) => s.is_stale)) { - map[channel.id] = 'partially-stale-streams-row'; + map[channel.id] = 'has-stale-streams-row'; } } return map; diff --git a/frontend/src/components/tables/table.css b/frontend/src/components/tables/table.css index 0e02dc1e..1e21540a 100644 --- a/frontend/src/components/tables/table.css +++ b/frontend/src/components/tables/table.css @@ -140,14 +140,14 @@ html { background-color: color-mix(in srgb, rgb(239, 68, 68) 30%, #27272a) !important; } -/* Rows that contain some stale streams (may be mixed with active) */ -.partially-stale-streams-row { +/* Rows that contain at least 1 stale stream (may be mixed with active) */ +.has-stale-streams-row { background-color: rgba(239, 154, 68, 0.15) !important; /* orange-500 @ 12% */ box-shadow: inset 0 0 0 1px rgba(239, 154, 68, 0.3); } -.table-striped .tbody .tr.partially-stale-streams-row:hover { +.table-striped .tbody .tr.has-stale-streams-row:hover { background-color: rgba(239, 154, 68, 0.3) !important; } From 6d7130d71e4d2f70df8999dbb29ff6cbc4c607a0 Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 11:06:52 -0600 Subject: [PATCH 25/58] Bug Fix: fix get_instance deadlock and non-atomic ownership acquisition If ProxyServer() raises during singleton construction, _instance was left as _INITIALIZING permanently, causing all subsequent get_instance() callers to spin in an infinite gevent.sleep() loop. Wrap construction in try/except and reset _instance to None on failure so callers can retry. Also replace the non-atomic setnx() + expire() pair in try_acquire_ownership() with a single atomic SET NX EX call, consistent with the approach already used in extend_ownership() and eliminating the race window where a crash between the two calls could leave a key with no TTL (permanent ownership lock). --- apps/proxy/ts_proxy/server.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/proxy/ts_proxy/server.py b/apps/proxy/ts_proxy/server.py index b0b1a08e..609a3000 100644 --- a/apps/proxy/ts_proxy/server.py +++ b/apps/proxy/ts_proxy/server.py @@ -43,13 +43,17 @@ class ProxyServer: return inst if inst is None: cls._instance = cls._INITIALIZING - from .server import ProxyServer - from .stream_manager import StreamManager - from .stream_buffer import StreamBuffer - from .client_manager import ClientManager - real_instance = ProxyServer() - cls._instance = real_instance - return real_instance + try: + from .server import ProxyServer + from .stream_manager import StreamManager + from .stream_buffer import StreamBuffer + from .client_manager import ClientManager + real_instance = ProxyServer() + cls._instance = real_instance + return real_instance + except Exception: + cls._instance = None # Reset so next call can retry + raise # Another greenlet is initializing — wait for completion while True: inst = cls._instance @@ -391,20 +395,16 @@ class ProxyServer: # Create a lock key with proper namespace lock_key = RedisKeys.channel_owner(channel_id) - # Use Redis SETNX for atomic locking with error handling + # Use atomic SET NX EX for locking with error handling acquired = self._execute_redis_command( - lambda: self.redis_client.setnx(lock_key, self.worker_id) + lambda: self.redis_client.set(lock_key, self.worker_id, nx=True, ex=ttl) ) if acquired is None: # Redis command failed logger.warning(f"Redis command failed during ownership acquisition - assuming ownership") return True - # If acquired, set expiry to prevent orphaned locks if acquired: - self._execute_redis_command( - lambda: self.redis_client.expire(lock_key, ttl) - ) logger.info(f"Worker {self.worker_id} acquired ownership of channel {channel_id}") return True From d6e4a34f3b78f14a480db00203576b3e826931fb Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 11:11:50 -0600 Subject: [PATCH 26/58] changelog: Update changelog for proxy server pr. --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6606c9b..fadc44e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Channel table filter for channels that have stale streams: A new "Has Stale Streams" filter option in the channel table header menu highlights and filters channels containing at least one stale stream. Channels with stale streams are visually distinguished with an orange tint. The filter is mutually exclusive with "Only Empty Channels". - Thanks [@JCBird1012](https://github.com/JCBird1012) +### Fixed + +- TS proxy streams dying after 30–200 seconds in multi-worker uWSGI/Celery deployments, caused by three interrelated bugs. (Fixes #992, #980) - Thanks [@PFalko](https://github.com/PFalko) + - **Double ProxyServer instantiation**: `ProxyConfig.ready()` called `TSProxyServer()` directly while `TSProxyConfig.ready()` also called `TSProxyServer.get_instance()`, creating two instances per worker — each with its own cleanup thread. The orphaned thread could not extend ownership because it had no entries in `stream_managers`. Fixed by using `TSProxyServer.get_instance()` in `ProxyConfig.ready()`. + - **`flushdb()` on every Redis client init**: `RedisClient.get_client()` called `client.flushdb()` whenever `_client` was `None`. Celery autoscale (`--autoscale=6,1`) spawning new workers mid-stream triggered this path, nuking all Redis keys including active ownership keys, client records, and channel metadata. Removed the `flushdb()` call entirely. + - **No recovery from expired ownership**: `get_channel_owner()` called `redis.get()` twice inside a lambda (TOCTOU race — key could expire between calls); `extend_ownership()` silently returned `False` on expiry with no re-acquisition; and the non-owner cleanup path unconditionally killed streams even when the worker held the `stream_manager`. Fixed with a single `GET` in `get_channel_owner()`, re-acquisition via atomic `SET NX EX` in `extend_ownership()`, and a re-acquisition attempt with client-aware cleanup deferral in the cleanup thread. +- `get_instance()` deadlock: if `ProxyServer()` raised an exception during singleton construction, `_instance` was left permanently as the `_INITIALIZING` sentinel, causing all subsequent `get_instance()` callers to spin in an infinite `gevent.sleep()` loop. Construction is now wrapped in `try/except`; on failure `_instance` resets to `None` so the next call can retry. +- Non-atomic ownership acquisition in `try_acquire_ownership()`: replaced the separate `setnx()` + `expire()` calls with a single atomic `SET NX EX`, eliminating the race window where a process crash between the two calls could leave an ownership key with no TTL (permanent ownership lock). + ## [0.20.2] - 2026-03-03 ### Security From dcb3cf8ea866a9e4928e476bbb5398b2e979bb6f Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 11:56:06 -0600 Subject: [PATCH 27/58] changelog: Update changelog for monkey patch PR. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fadc44e8..55fac12c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Celery Redis connections using unpatched sockets under gevent: `dispatcharr/__init__.py` imports `celery.py` as part of `django.setup()`, initialising the Redis connection pool before uWSGI's gevent plugin patches stdlib sockets. Added `gevent.monkey.patch_all()` at the top of `wsgi.py` to guarantee all broker connections are gevent-aware. - Thanks [@endoze](https://github.com/endoze) - TS proxy streams dying after 30–200 seconds in multi-worker uWSGI/Celery deployments, caused by three interrelated bugs. (Fixes #992, #980) - Thanks [@PFalko](https://github.com/PFalko) - **Double ProxyServer instantiation**: `ProxyConfig.ready()` called `TSProxyServer()` directly while `TSProxyConfig.ready()` also called `TSProxyServer.get_instance()`, creating two instances per worker — each with its own cleanup thread. The orphaned thread could not extend ownership because it had no entries in `stream_managers`. Fixed by using `TSProxyServer.get_instance()` in `ProxyConfig.ready()`. - **`flushdb()` on every Redis client init**: `RedisClient.get_client()` called `client.flushdb()` whenever `_client` was `None`. Celery autoscale (`--autoscale=6,1`) spawning new workers mid-stream triggered this path, nuking all Redis keys including active ownership keys, client records, and channel metadata. Removed the `flushdb()` call entirely. From 7653859a9a268ba0d9d5aac56152d79800516f16 Mon Sep 17 00:00:00 2001 From: None Date: Tue, 3 Mar 2026 14:22:45 -0600 Subject: [PATCH 28/58] Reset connection_allocated flag after redirect stream release The redirect path in stream_ts() released the connection slot via release_stream() but did not reset the connection_allocated flag. Could have could caused a redundant no-op call in the exception handler. --- apps/proxy/ts_proxy/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/proxy/ts_proxy/views.py b/apps/proxy/ts_proxy/views.py index 90ac2673..ccbb947d 100644 --- a/apps/proxy/ts_proxy/views.py +++ b/apps/proxy/ts_proxy/views.py @@ -327,6 +327,7 @@ def stream_ts(request, channel_id): # Release stream lock before redirecting if not channel.release_stream(): logger.warning(f"[{client_id}] Failed to release stream before redirect") + connection_allocated = False # Final decision based on validation results if is_valid: logger.info( From e78c24aad735952eb6f8939dc2935aa4fe20546d Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 15:13:46 -0600 Subject: [PATCH 29/58] Enhance Redis chunk management in StreamBuffer and StreamGenerator - Register Lua scripts for optimized chunk retrieval in StreamBuffer. - Implement atomic Lua binary search to find the oldest available chunk in Redis. - Update StreamGenerator to handle expired chunks more efficiently, reducing TOCTOU race conditions. --- apps/proxy/ts_proxy/stream_buffer.py | 92 +++++++++++++++++++++++++ apps/proxy/ts_proxy/stream_generator.py | 36 +++++++--- 2 files changed, 119 insertions(+), 9 deletions(-) diff --git a/apps/proxy/ts_proxy/stream_buffer.py b/apps/proxy/ts_proxy/stream_buffer.py index 85feb5dd..2bf99527 100644 --- a/apps/proxy/ts_proxy/stream_buffer.py +++ b/apps/proxy/ts_proxy/stream_buffer.py @@ -45,6 +45,15 @@ class StreamBuffer: self._write_buffer = bytearray() self.target_chunk_size = ConfigHelper.get('BUFFER_CHUNK_SIZE', TS_PACKET_SIZE * 5644) # ~1MB default + # Register Lua scripts once — subsequent calls use EVALSHA (just the + # SHA hash) instead of sending the full script text on every invocation. + if self.redis_client: + self._find_oldest_chunk_sha = self.redis_client.register_script( + self._FIND_OLDEST_CHUNK_LUA + ) + else: + self._find_oldest_chunk_sha = None + # Track timers for proper cleanup self.stopping = False self.fill_timers = [] @@ -328,6 +337,89 @@ class StreamBuffer: return chunks, client_index + chunk_count + # Lua script that runs an atomic binary search on the Redis server. + # Chunks expire in FIFO order (same TTL, sequential writes), so the + # alive range is contiguous: [oldest_surviving .. buffer_head]. + # Binary search finds the boundary in O(log N) EXISTS calls with zero + # round-trips between steps and no TOCTOU races (Lua scripts are atomic). + # + # ARGV[1] = key prefix (e.g. "ts_proxy:channel::buffer:chunk:") + # ARGV[2] = low index (client_index + 1, first chunk the client needs) + # ARGV[3] = high index (buffer head, most recent chunk) + # + # Returns: the index of the oldest existing chunk, or -1 if none exist. + _FIND_OLDEST_CHUNK_LUA = """ + local prefix = ARGV[1] + local low = tonumber(ARGV[2]) + local high = tonumber(ARGV[3]) + + if redis.call('EXISTS', prefix .. high) == 0 then + return -1 + end + + local result = high + while low <= high do + local mid = math.floor((low + high) / 2) + if redis.call('EXISTS', prefix .. mid) == 1 then + result = mid + high = mid - 1 + else + low = mid + 1 + end + end + return result + """ + + def find_oldest_available_chunk(self, client_index): + """Find the oldest (lowest-index) chunk that still exists in Redis. + + Executes an atomic Lua binary search on the Redis server — one + round-trip, ~log2(N) EXISTS calls, no TOCTOU between steps. + + The actual read attempt (get_optimized_client_data) is what + authoritatively detects expiration; this method is best-effort + positioning that self-corrects on the next iteration if the found + chunk also expires before the client can read it. + + Args: + client_index: The client's current local_index (last consumed chunk). + + Returns: + int or None: The local_index value the client should jump to + (one before the first available chunk), or None if no + chunks are available at all. + """ + if not self.redis_client: + return None + + low = client_index + 1 # First chunk the client needs + high = self.index # Latest chunk written + + if low > high: + return None + + try: + # Uses EVALSHA under the hood — sends only the SHA hash, + # not the full script text, on every call after the first. + result = self._find_oldest_chunk_sha( + args=[ + RedisKeys.buffer_chunk_prefix(self.channel_id), + low, + high, + ], + ) + + if result == -1: + return None + + # Return result - 1 so local_index points to one before the + # first available chunk (matching the "last consumed" convention). + return int(result) - 1 + + except Exception as e: + logger.error(f"Error running find_oldest_chunk Lua script for channel {self.channel_id}: {e}") + return None + # Add a new method to safely create timers def schedule_timer(self, delay, callback, *args, **kwargs): """Schedule a timer and track it for proper cleanup""" diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 50404f1d..8f631d53 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -224,17 +224,35 @@ class StreamGenerator: self.empty_reads += 1 self.consecutive_empty += 1 - # Check if we're too far behind (chunks expired from Redis) + # We got no data despite being behind the buffer head. + # The read itself is the authoritative signal — no separate + # existence check needed, avoiding TOCTOU races with Redis TTL. chunks_behind = self.buffer.index - self.local_index - if chunks_behind > 50: # If more than 50 chunks behind, jump forward - # Calculate new position: stay a few chunks behind current buffer - initial_behind = ConfigHelper.initial_behind_chunks() - new_index = max(self.local_index, self.buffer.index - initial_behind) + if chunks_behind > 0: + # Next chunk has expired — find the oldest chunk still in Redis + new_index = self.buffer.find_oldest_available_chunk(self.local_index) - logger.warning(f"[{self.client_id}] Client too far behind ({chunks_behind} chunks), jumping from {self.local_index} to {new_index}") - self.local_index = new_index - self.consecutive_empty = 0 # Reset since we're repositioning - continue # Try again immediately with new position + if new_index is not None: + skipped = new_index - self.local_index + logger.warning( + f"[{self.client_id}] Next chunk expired (index {self.local_index + 1}), " + f"jumping to oldest available: {new_index + 1} " + f"(skipped {skipped} chunks, buffer head at {self.buffer.index})" + ) + self.local_index = new_index + else: + # No chunks available at all — jump to near the buffer head + initial_behind = ConfigHelper.initial_behind_chunks() + new_index = max(self.local_index, self.buffer.index - initial_behind) + logger.warning( + f"[{self.client_id}] No chunks available in buffer, " + f"jumping to near buffer head: {new_index} " + f"(buffer head at {self.buffer.index})" + ) + self.local_index = new_index + + self.consecutive_empty = 0 + continue # Retry immediately with the new position if self._should_send_keepalive(self.local_index): keepalive_packet = create_ts_packet('keepalive') From d487bb5671236877c28dfe1e148e7072c9a15fe2 Mon Sep 17 00:00:00 2001 From: SergeantPanda <61642231+SergeantPanda@users.noreply.github.com> Date: Tue, 3 Mar 2026 15:33:55 -0600 Subject: [PATCH 30/58] Revert "fix(wsgi): ensure gevent monkey-patching before Django import chain" --- CHANGELOG.md | 1 - dispatcharr/tests/__init__.py | 0 dispatcharr/tests/test_wsgi.py | 39 ---------------------------------- dispatcharr/wsgi.py | 15 ------------- 4 files changed, 55 deletions(-) delete mode 100644 dispatcharr/tests/__init__.py delete mode 100644 dispatcharr/tests/test_wsgi.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 55fac12c..fadc44e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Celery Redis connections using unpatched sockets under gevent: `dispatcharr/__init__.py` imports `celery.py` as part of `django.setup()`, initialising the Redis connection pool before uWSGI's gevent plugin patches stdlib sockets. Added `gevent.monkey.patch_all()` at the top of `wsgi.py` to guarantee all broker connections are gevent-aware. - Thanks [@endoze](https://github.com/endoze) - TS proxy streams dying after 30–200 seconds in multi-worker uWSGI/Celery deployments, caused by three interrelated bugs. (Fixes #992, #980) - Thanks [@PFalko](https://github.com/PFalko) - **Double ProxyServer instantiation**: `ProxyConfig.ready()` called `TSProxyServer()` directly while `TSProxyConfig.ready()` also called `TSProxyServer.get_instance()`, creating two instances per worker — each with its own cleanup thread. The orphaned thread could not extend ownership because it had no entries in `stream_managers`. Fixed by using `TSProxyServer.get_instance()` in `ProxyConfig.ready()`. - **`flushdb()` on every Redis client init**: `RedisClient.get_client()` called `client.flushdb()` whenever `_client` was `None`. Celery autoscale (`--autoscale=6,1`) spawning new workers mid-stream triggered this path, nuking all Redis keys including active ownership keys, client records, and channel metadata. Removed the `flushdb()` call entirely. diff --git a/dispatcharr/tests/__init__.py b/dispatcharr/tests/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/dispatcharr/tests/test_wsgi.py b/dispatcharr/tests/test_wsgi.py deleted file mode 100644 index cdf7a2e9..00000000 --- a/dispatcharr/tests/test_wsgi.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Tests for dispatcharr/wsgi.py gevent monkey-patching.""" - -import os -import subprocess -import sys -import unittest - - -class TestWSGIGeventPatching(unittest.TestCase): - """Verify that wsgi.py applies gevent monkey-patching before Django imports. - - These tests run in subprocesses because monkey.patch_all() must execute - before any other imports. By the time Django's test runner loads this - file, ssl/socket are already imported, so calling patch_all() in-process - would fail. A subprocess mirrors how uWSGI actually loads wsgi.py. - """ - - def test_socket_is_patched_after_wsgi_import(self): - """Loading wsgi.py first (as uWSGI does) should patch sockets.""" - result = subprocess.run( - [ - sys.executable, "-c", - "import dispatcharr.wsgi; " - "from gevent import monkey; " - "assert monkey.is_module_patched('socket'), " - "'socket module was not patched by wsgi.py'; " - "print('PASS')", - ], - capture_output=True, - text=True, - env={**os.environ, "DJANGO_SETTINGS_MODULE": "dispatcharr.settings"}, - timeout=60, - cwd="/app", - ) - self.assertEqual( - result.returncode, - 0, - f"monkey-patching check failed:\n{result.stderr}", - ) diff --git a/dispatcharr/wsgi.py b/dispatcharr/wsgi.py index 6e8860af..29c2ba01 100644 --- a/dispatcharr/wsgi.py +++ b/dispatcharr/wsgi.py @@ -1,21 +1,6 @@ """ WSGI config for dispatcharr project. """ -# When running under uWSGI with gevent, ensure monkey-patching is fully -# applied before any other imports. Django's setup triggers the import of -# dispatcharr/__init__.py → celery.py, which initialises the Celery broker -# transport. If those imports happen before sockets are patched, Celery's -# Redis connection pool holds unpatched sockets that silently fail under -# gevent's cooperative scheduling. Calling patch_all() here—before the -# Django import chain—guarantees every socket (including pooled broker -# connections) is gevent-aware. The call is idempotent, so it's harmless -# if uWSGI's gevent plugin has already patched. -try: - from gevent import monkey - monkey.patch_all() -except ImportError: - pass - import os from django.core.wsgi import get_wsgi_application From 24c20a2f4accf486b89c7e664d92571d2cdc845b Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 15:59:49 -0600 Subject: [PATCH 31/58] changelog: Update changelog for proxy changes. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fadc44e8..49e7be68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- TS proxy client stream lag recovery now only bumps clients forward when their next required chunk has genuinely expired from Redis (TTL), rather than unconditionally jumping if they fell more than 50 chunks behind. Clients are repositioned to the oldest available chunk (minimum data loss) using an atomic server-side Lua binary search, falling back to near the buffer head if nothing is available. - TS proxy streams dying after 30–200 seconds in multi-worker uWSGI/Celery deployments, caused by three interrelated bugs. (Fixes #992, #980) - Thanks [@PFalko](https://github.com/PFalko) - **Double ProxyServer instantiation**: `ProxyConfig.ready()` called `TSProxyServer()` directly while `TSProxyConfig.ready()` also called `TSProxyServer.get_instance()`, creating two instances per worker — each with its own cleanup thread. The orphaned thread could not extend ownership because it had no entries in `stream_managers`. Fixed by using `TSProxyServer.get_instance()` in `ProxyConfig.ready()`. - **`flushdb()` on every Redis client init**: `RedisClient.get_client()` called `client.flushdb()` whenever `_client` was `None`. Celery autoscale (`--autoscale=6,1`) spawning new workers mid-stream triggered this path, nuking all Redis keys including active ownership keys, client records, and channel metadata. Removed the `flushdb()` call entirely. From 086259ef9dfad69c83efe11379a12536b652d29a Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 16:28:30 -0600 Subject: [PATCH 32/58] changelog: Update changelog for stream lock PR. --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e7be68..be8b88b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- TS proxy connection slot leaks and TOCTOU races in stream initialization (Fixes #947) - Thanks [@CodeBormen](https://github.com/CodeBormen) + - **TOCTOU race in slot reservation**: `get_stream()` previously used a `GET`→check→`INCR` sequence, allowing concurrent requests to both read the same count below the limit and both reserve a slot, silently exceeding `max_streams`. Replaced with an atomic `INCR`-first pattern: increment unconditionally, check the result, roll back with `DECR` if over capacity. + - **Leak on URL generation failure**: `generate_stream_url()` called `get_stream()` (which `INCR`s the counter) but had no cleanup path if subsequent DB lookups or URL construction failed. The post-`get_stream()` block is now wrapped in a `try/except` that calls `release_stream()` on any error. + - **Leak on retry-loop timeout**: the retry loop in `stream_ts()` called a bare `get_stream()` on the first failure to classify the error reason. If a slot was available, this `INCR`'d the counter and set Redis keys that were never released when the loop timed out. A `release_stream()` call is now issued before returning 503. + - **Leak on `initialize_channel()` failure**: when `initialize_channel()` returned `False`, the connection slot allocated by the preceding `get_stream()` was never released. A `connection_allocated` flag now tracks whether this request performed the `INCR` (fresh initialization vs. joining an existing channel), and `release_stream()` is called guarded by that flag to prevent incorrect decrements when attaching to an already-running channel. + - **Safety net for unexpected exceptions**: the outer `except` in `stream_ts()` now checks `connection_allocated` and calls `release_stream()` as a last-resort cleanup for any unhandled exception that escapes before the channel is handed off to the stream lifecycle. + - **`release_stream()` now returns `bool`** and adds a metadata-hash fallback: if the primary `channel_stream` / `stream_profile` Redis keys have already been cleaned up by the proxy, it recovers `stream_id` and `profile_id` from the channel's metadata hash and clears those fields atomically to prevent duplicate `DECR`s on repeated calls. + - **`update_stream_profile()` uses a Redis pipeline** for the old-profile DECR + key update + new-profile INCR sequence, preventing counter drift if the process crashes between operations. + - **`stream_generator._cleanup()`** now falls back to `Stream.objects.get()` when the channel UUID resolves to a preview flow rather than a normal channel, rather than silently skipping the slot release. + - **VOD `cleanup_persistent_connection()`** fallback DECR is now conditional: it only decrements the profile counter when the connection tracking key had already expired by TTL (i.e., `remove_connection()` would have skipped the DECR), preventing double-decrements when the key is still present. - TS proxy client stream lag recovery now only bumps clients forward when their next required chunk has genuinely expired from Redis (TTL), rather than unconditionally jumping if they fell more than 50 chunks behind. Clients are repositioned to the oldest available chunk (minimum data loss) using an atomic server-side Lua binary search, falling back to near the buffer head if nothing is available. - TS proxy streams dying after 30–200 seconds in multi-worker uWSGI/Celery deployments, caused by three interrelated bugs. (Fixes #992, #980) - Thanks [@PFalko](https://github.com/PFalko) - **Double ProxyServer instantiation**: `ProxyConfig.ready()` called `TSProxyServer()` directly while `TSProxyConfig.ready()` also called `TSProxyServer.get_instance()`, creating two instances per worker — each with its own cleanup thread. The orphaned thread could not extend ownership because it had no entries in `stream_managers`. Fixed by using `TSProxyServer.get_instance()` in `ProxyConfig.ready()`. From fc75a68195b59d1f00d1df2994c8741f443a6d76 Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 18:12:32 -0600 Subject: [PATCH 33/58] =?UTF-8?q?Enhancement:=20new=20clients=20joining=20?= =?UTF-8?q?an=20active=20channel=20are=20now=20positioned=20a=20configurab?= =?UTF-8?q?le=20number=20of=20seconds=20behind=20live=20rather=20than=20a?= =?UTF-8?q?=20fixed=20chunk=20count.=20The=20start=20position=20is=20deter?= =?UTF-8?q?mined=20by=20wall-clock=20chunk=20receive=20time=20(stored=20as?= =?UTF-8?q?=20a=20Redis=20sorted=20set=20alongside=20the=20buffer),=20so?= =?UTF-8?q?=20the=20buffer=20depth=20is=20consistent=20in=20seconds=20rega?= =?UTF-8?q?rdless=20of=20stream=20bitrate.=20Setting=20the=20value=20to=20?= =?UTF-8?q?`0`=20starts=20clients=20at=20live=20with=20no=20buffer.=20Defa?= =?UTF-8?q?ults=20to=202=20seconds.=20Existing=20chunk-count=20gating=20fo?= =?UTF-8?q?r=20the=20first=20client=20connecting=20to=20a=20channel=20is?= =?UTF-8?q?=20unchanged.=20The=20setting=20is=20exposed=20in=20Settings=20?= =?UTF-8?q?=E2=86=92=20Proxy=20as=20"New=20Client=20Buffer=20(seconds)".?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 1 + apps/proxy/config.py | 2 + apps/proxy/ts_proxy/config_helper.py | 9 +++ apps/proxy/ts_proxy/redis_keys.py | 6 ++ apps/proxy/ts_proxy/stream_buffer.py | 79 +++++++++++++++++++ apps/proxy/ts_proxy/stream_generator.py | 43 +++++++++- core/api_views.py | 1 + core/models.py | 1 + core/serializers.py | 6 ++ .../forms/settings/ProxySettingsForm.jsx | 12 ++- frontend/src/constants.js | 5 ++ .../forms/settings/ProxySettingsFormUtils.js | 1 + 12 files changed, 161 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be8b88b4..88f6797a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- New Client Buffer proxy setting: new clients joining an active channel are now positioned a configurable number of seconds behind live rather than a fixed chunk count. The start position is determined by wall-clock chunk receive time (stored as a Redis sorted set alongside the buffer), so the buffer depth is consistent in seconds regardless of stream bitrate. Setting the value to `0` starts clients at live with no buffer. Defaults to 2 seconds. Existing chunk-count gating for the first client connecting to a channel is unchanged. The setting is exposed in Settings → Proxy as "New Client Buffer (seconds)". - Channel table filter for channels that have stale streams: A new "Has Stale Streams" filter option in the channel table header menu highlights and filters channels containing at least one stale stream. Channels with stale streams are visually distinguished with an orange tint. The filter is mutually exclusive with "Only Empty Channels". - Thanks [@JCBird1012](https://github.com/JCBird1012) ### Fixed diff --git a/apps/proxy/config.py b/apps/proxy/config.py index 3b1ce967..dbdd8278 100644 --- a/apps/proxy/config.py +++ b/apps/proxy/config.py @@ -43,6 +43,7 @@ class BaseConfig: "redis_chunk_ttl": 60, "channel_shutdown_delay": 0, "channel_init_grace_period": 5, + "new_client_behind_seconds": 2, } finally: @@ -81,6 +82,7 @@ class TSConfig(BaseConfig): # Buffer settings INITIAL_BEHIND_CHUNKS = 4 # How many chunks behind to start a client (4 chunks = ~1MB) CHUNK_BATCH_SIZE = 5 # How many chunks to fetch in one batch + NEW_CLIENT_BEHIND_SECONDS = 2 # Start new clients this many seconds behind live (0 = start at live) KEEPALIVE_INTERVAL = 0.5 # Seconds between keepalive packets when at buffer head # Chunk read timeout CHUNK_TIMEOUT = 5 # Seconds to wait for each chunk read diff --git a/apps/proxy/ts_proxy/config_helper.py b/apps/proxy/ts_proxy/config_helper.py index d7d33558..f0f7a5b7 100644 --- a/apps/proxy/ts_proxy/config_helper.py +++ b/apps/proxy/ts_proxy/config_helper.py @@ -41,6 +41,15 @@ class ConfigHelper: """Get number of chunks to start behind""" return ConfigHelper.get('INITIAL_BEHIND_CHUNKS', 4) + @staticmethod + def new_client_behind_seconds(): + """Get number of seconds behind live to start new clients. + 0 means start at live (buffer head). + Loaded from DB proxy_settings so users can change it at runtime.""" + from apps.proxy.config import TSConfig + settings = TSConfig.get_proxy_settings() + return settings.get('new_client_behind_seconds', 2) + @staticmethod def keepalive_interval(): """Get keepalive interval in seconds""" diff --git a/apps/proxy/ts_proxy/redis_keys.py b/apps/proxy/ts_proxy/redis_keys.py index 22b02648..29846f98 100644 --- a/apps/proxy/ts_proxy/redis_keys.py +++ b/apps/proxy/ts_proxy/redis_keys.py @@ -79,6 +79,12 @@ class RedisKeys: """Key for worker heartbeat""" return f"ts_proxy:worker:{worker_id}:heartbeat" + @staticmethod + def chunk_timestamps(channel_id): + """Sorted set mapping chunk receive-timestamps (score) to chunk indices (member). + Used for time-based client positioning.""" + return f"ts_proxy:channel:{channel_id}:buffer:chunk_timestamps" + @staticmethod def transcode_active(channel_id): """Key indicating active transcode process""" diff --git a/apps/proxy/ts_proxy/stream_buffer.py b/apps/proxy/ts_proxy/stream_buffer.py index 2bf99527..111468bf 100644 --- a/apps/proxy/ts_proxy/stream_buffer.py +++ b/apps/proxy/ts_proxy/stream_buffer.py @@ -45,14 +45,21 @@ class StreamBuffer: self._write_buffer = bytearray() self.target_chunk_size = ConfigHelper.get('BUFFER_CHUNK_SIZE', TS_PACKET_SIZE * 5644) # ~1MB default + # Sorted-set key for chunk receive-timestamps (time-based positioning) + self.chunk_timestamps_key = RedisKeys.chunk_timestamps(channel_id) if channel_id else "" + # Register Lua scripts once — subsequent calls use EVALSHA (just the # SHA hash) instead of sending the full script text on every invocation. if self.redis_client: self._find_oldest_chunk_sha = self.redis_client.register_script( self._FIND_OLDEST_CHUNK_LUA ) + self._find_chunk_by_time_sha = self.redis_client.register_script( + self._FIND_CHUNK_BY_TIME_LUA + ) else: self._find_oldest_chunk_sha = None + self._find_chunk_by_time_sha = None # Track timers for proper cleanup self.stopping = False @@ -101,6 +108,14 @@ class StreamBuffer: chunk_key = RedisKeys.buffer_chunk(self.channel_id, chunk_index) self.redis_client.setex(chunk_key, self.chunk_ttl, bytes(chunk_data)) + # Record receive timestamp for time-based client positioning + if self.chunk_timestamps_key: + now = time.time() + self.redis_client.zadd(self.chunk_timestamps_key, {str(chunk_index): now}) + # Prune entries whose chunks have expired from Redis + self.redis_client.zremrangebyscore(self.chunk_timestamps_key, '-inf', now - self.chunk_ttl) + self.redis_client.expire(self.chunk_timestamps_key, self.chunk_ttl) + # Update local tracking self.index = chunk_index writes_done += 1 @@ -284,6 +299,13 @@ class StreamBuffer: if hasattr(self, '_partial_packet'): self._partial_packet = bytearray() + # Clean up the chunk timestamps sorted set + if self.redis_client and self.chunk_timestamps_key: + try: + self.redis_client.delete(self.chunk_timestamps_key) + except Exception as e: + logger.error(f"Error deleting chunk timestamps key: {e}") + except Exception as e: logger.error(f"Error during buffer stop: {e}") @@ -420,6 +442,63 @@ class StreamBuffer: logger.error(f"Error running find_oldest_chunk Lua script for channel {self.channel_id}: {e}") return None + # ------------------------------------------------------------------ + # Lua script: atomic reverse-scan of the chunk_timestamps sorted set. + # Finds the chunk whose receive-timestamp is closest to (but <=) a + # target wall-clock time. Returns the chunk index or -1. + # + # KEYS[1] = chunk_timestamps sorted-set key + # ARGV[1] = target timestamp (time.time() - desired_seconds_behind) + # ------------------------------------------------------------------ + _FIND_CHUNK_BY_TIME_LUA = """ + local ts_key = KEYS[1] + local target = tonumber(ARGV[1]) + + -- ZREVRANGEBYSCORE returns members with score <= target, highest first. + local result = redis.call('ZREVRANGEBYSCORE', ts_key, target, '-inf', 'LIMIT', 0, 1) + if #result == 0 then + return -1 + end + return tonumber(result[1]) + """ + + def find_chunk_index_by_time(self, seconds_behind): + """Find the chunk index that was received approximately *seconds_behind* + seconds ago. + + Uses an atomic Lua script against the chunk_timestamps sorted set so + no data can expire between the lookup and the read. + + Returns: + int or None: The chunk index to position the client at (this is + the *last consumed* convention, so the next read + starts at index+1). None if no suitable chunk + exists. + """ + if not self.redis_client or not self.chunk_timestamps_key: + return None + + target_time = time.time() - seconds_behind + + try: + result = self._find_chunk_by_time_sha( + keys=[self.chunk_timestamps_key], + args=[target_time], + ) + if result is None or int(result) == -1: + # No chunk old enough — fall back to the oldest available chunk + oldest = self.redis_client.zrange(self.chunk_timestamps_key, 0, 0) + if oldest: + return max(0, int(oldest[0]) - 1) # "last consumed" convention + return None + + # Return index - 1 so next read starts at that chunk + return max(0, int(result) - 1) + + except Exception as e: + logger.error(f"Error in find_chunk_index_by_time for channel {self.channel_id}: {e}") + return None + # Add a new method to safely create timers def schedule_timer(self, delay, callback, *args, **kwargs): """Schedule a timer and track it for proper cleanup""" diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index a8ee799f..05892293 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -186,10 +186,47 @@ class StreamGenerator: logger.error(f"[{self.client_id}] No buffer found for channel {self.channel_id}") return False - # Client state tracking - use config for initial position - initial_behind = ConfigHelper.initial_behind_chunks() + # Client state tracking — determine start position + # When behind_seconds > 0, use time-based positioning to start + # the client that many seconds behind live. + # When behind_seconds == 0, start at live (buffer head). + behind_seconds = ConfigHelper.new_client_behind_seconds() current_buffer_index = buffer.index - self.local_index = max(0, current_buffer_index - initial_behind) + + if behind_seconds > 0: + time_index = buffer.find_chunk_index_by_time(behind_seconds) + if time_index is not None: + self.local_index = max(0, time_index) + logger.info( + f"[{self.client_id}] Time-based positioning: " + f"{behind_seconds}s behind -> index {self.local_index} " + f"(buffer head at {current_buffer_index})" + ) + else: + # Not enough buffer for the requested time — start as far + # back as possible (oldest available chunk). + oldest = buffer.find_oldest_available_chunk(0) + if oldest is not None: + self.local_index = max(0, oldest) + logger.info( + f"[{self.client_id}] Buffer shorter than {behind_seconds}s, " + f"starting at oldest available chunk {self.local_index} " + f"(buffer head at {current_buffer_index})" + ) + else: + # No timestamp data at all — start at live + self.local_index = current_buffer_index + logger.info( + f"[{self.client_id}] No timestamp data, starting at live: " + f"index {self.local_index} (buffer head at {current_buffer_index})" + ) + else: + # 0 = start at live (buffer head) + self.local_index = current_buffer_index + logger.info( + f"[{self.client_id}] Starting at live (behind_seconds=0): " + f"index {self.local_index} (buffer head at {current_buffer_index})" + ) # Store important objects as instance variables self.buffer = buffer diff --git a/core/api_views.py b/core/api_views.py index 3d131ab9..43e55d02 100644 --- a/core/api_views.py +++ b/core/api_views.py @@ -183,6 +183,7 @@ class ProxySettingsViewSet(viewsets.ViewSet): "redis_chunk_ttl": 60, "channel_shutdown_delay": 0, "channel_init_grace_period": 5, + "new_client_behind_seconds": 2, } settings_obj, created = CoreSettings.objects.get_or_create( key=PROXY_SETTINGS_KEY, diff --git a/core/models.py b/core/models.py index 10f9073f..7ce7713d 100644 --- a/core/models.py +++ b/core/models.py @@ -329,6 +329,7 @@ class CoreSettings(models.Model): "redis_chunk_ttl": 60, "channel_shutdown_delay": 0, "channel_init_grace_period": 5, + "new_client_behind_seconds": 2, }) # System Settings diff --git a/core/serializers.py b/core/serializers.py index 03ba33e8..b0b730ee 100644 --- a/core/serializers.py +++ b/core/serializers.py @@ -78,6 +78,7 @@ class ProxySettingsSerializer(serializers.Serializer): redis_chunk_ttl = serializers.IntegerField(min_value=10, max_value=3600) channel_shutdown_delay = serializers.IntegerField(min_value=0, max_value=300) channel_init_grace_period = serializers.IntegerField(min_value=0, max_value=60) + new_client_behind_seconds = serializers.IntegerField(min_value=0, max_value=120, required=False, default=2) def validate_buffering_timeout(self, value): if value < 0 or value > 300: @@ -104,6 +105,11 @@ class ProxySettingsSerializer(serializers.Serializer): raise serializers.ValidationError("Channel init grace period must be between 0 and 60 seconds") return value + def validate_new_client_behind_seconds(self, value): + if value < 0 or value > 120: + raise serializers.ValidationError("New client buffer must be between 0 and 120 seconds") + return value + class SystemNotificationSerializer(serializers.ModelSerializer): """Serializer for system notifications.""" diff --git a/frontend/src/components/forms/settings/ProxySettingsForm.jsx b/frontend/src/components/forms/settings/ProxySettingsForm.jsx index a5c8d858..52440769 100644 --- a/frontend/src/components/forms/settings/ProxySettingsForm.jsx +++ b/frontend/src/components/forms/settings/ProxySettingsForm.jsx @@ -24,6 +24,7 @@ const ProxySettingsOptions = React.memo(({ proxySettingsForm }) => { 'redis_chunk_ttl', 'channel_shutdown_delay', 'channel_init_grace_period', + 'new_client_behind_seconds', ].includes(key); }; const isFloatField = (key) => { @@ -36,7 +37,9 @@ const ProxySettingsOptions = React.memo(({ proxySettingsForm }) => { ? 3600 : key === 'channel_shutdown_delay' ? 300 - : 60; + : key === 'new_client_behind_seconds' + ? 120 + : 60; }; return ( <> @@ -97,7 +100,12 @@ const ProxySettingsForm = React.memo(({ active }) => { useEffect(() => { if (settings) { if (settings['proxy_settings']?.value) { - proxySettingsForm.setValues(settings['proxy_settings'].value); + // Merge defaults so any newly-added keys not yet in the stored + // settings object still show their default value rather than blank. + proxySettingsForm.setValues({ + ...getProxySettingDefaults(), + ...settings['proxy_settings'].value, + }); } } }, [settings]); diff --git a/frontend/src/constants.js b/frontend/src/constants.js index d2caa45b..0c19eea1 100644 --- a/frontend/src/constants.js +++ b/frontend/src/constants.js @@ -55,6 +55,11 @@ export const PROXY_SETTINGS_OPTIONS = { label: 'Channel Initialization Grace Period', description: 'Grace period in seconds during channel initialization', }, + new_client_behind_seconds: { + label: 'New Client Buffer (seconds)', + description: + 'Seconds of received buffer to start behind live when a new client connects (0 = start at live). Note: this is chunk receive time, not video duration.', + }, }; export const M3U_FILTER_TYPES = [ diff --git a/frontend/src/utils/forms/settings/ProxySettingsFormUtils.js b/frontend/src/utils/forms/settings/ProxySettingsFormUtils.js index 9ca185e8..e6e0464b 100644 --- a/frontend/src/utils/forms/settings/ProxySettingsFormUtils.js +++ b/frontend/src/utils/forms/settings/ProxySettingsFormUtils.js @@ -14,5 +14,6 @@ export const getProxySettingDefaults = () => { redis_chunk_ttl: 60, channel_shutdown_delay: 0, channel_init_grace_period: 5, + new_client_behind_seconds: 2, }; }; From b9a9d929cee8e642cf1fff62bfbda83cac242cdd Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Tue, 3 Mar 2026 18:21:52 -0600 Subject: [PATCH 34/58] tests: Fix proxy settings test. --- .../forms/settings/__tests__/ProxySettingsFormUtils.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/utils/forms/settings/__tests__/ProxySettingsFormUtils.test.js b/frontend/src/utils/forms/settings/__tests__/ProxySettingsFormUtils.test.js index 8feefccb..a48e5a37 100644 --- a/frontend/src/utils/forms/settings/__tests__/ProxySettingsFormUtils.test.js +++ b/frontend/src/utils/forms/settings/__tests__/ProxySettingsFormUtils.test.js @@ -61,6 +61,7 @@ describe('ProxySettingsFormUtils', () => { redis_chunk_ttl: 60, channel_shutdown_delay: 0, channel_init_grace_period: 5, + new_client_behind_seconds: 2, }); }); @@ -80,6 +81,7 @@ describe('ProxySettingsFormUtils', () => { expect(typeof result.redis_chunk_ttl).toBe('number'); expect(typeof result.channel_shutdown_delay).toBe('number'); expect(typeof result.channel_init_grace_period).toBe('number'); + expect(typeof result.new_client_behind_seconds).toBe('number'); }); }); }); From bd1e060bd213822055f3ed1758954bc8ce332a01 Mon Sep 17 00:00:00 2001 From: None Date: Thu, 26 Feb 2026 00:11:15 -0600 Subject: [PATCH 35/58] fix(dvr): fix duplicate recording execution and add manual stop Fixes #940: Duplicate DVR task executions were caused by ClockedSchedule objects being shared across PeriodicTasks, triggering run_recording multiple times for the same recording. Added an idempotency guard in run_recording that exits early if status is already 'recording', 'completed', or 'stopped'. revoke_task() now deletes the PeriodicTask and orphaned ClockedSchedule on execution rather than relying on Celery revocation alone. Fixes #454: Added POST /api/channels/recordings/{id}/stop/ action that marks the recording as stopped and terminates only the specific DVR proxy client (identified via User-Agent: Dispatcharr-DVR/recording-{id}) without disrupting simultaneous recordings on the same or other channels. The API returns immediately; DVR client teardown and task revocation happen in a background thread to avoid 504 timeouts on the recordings list endpoint. Additional changes: - destroy() now only calls _stop_dvr_clients() for in-progress recordings, preventing accidental stream termination when deleting completed recordings - WebSocket events differentiated: recording_stopped, recording_cancelled (in-progress cancel), and recording_cancelled (delete) with was_in_progress flag so the frontend shows distinct "Recording deleted" vs "Recording cancelled" notifications - Frontend: Stop and Cancel split into separate buttons with confirmation modals; stopped recordings move immediately to Previously Recorded via optimistic status bucketing; removed dead 'Stopped' badge state - 27 new unit tests covering scheduling, idempotency, revocation, and DVR client isolation --- apps/channels/api_views.py | 176 ++++-- apps/channels/signals.py | 98 +++- apps/channels/tasks.py | 241 ++++++-- .../tests/test_recording_scheduling.py | 547 ++++++++++++++++++ frontend/src/WebSocket.jsx | 28 + frontend/src/api.js | 11 + .../src/components/cards/RecordingCard.jsx | 125 +++- .../src/utils/cards/RecordingCardUtils.js | 4 + frontend/src/utils/pages/DVRUtils.js | 2 +- 9 files changed, 1112 insertions(+), 120 deletions(-) create mode 100644 apps/channels/tests/test_recording_scheduling.py diff --git a/apps/channels/api_views.py b/apps/channels/api_views.py index 9147532b..bd94b552 100644 --- a/apps/channels/api_views.py +++ b/apps/channels/api_views.py @@ -2248,6 +2248,54 @@ class RecurringRecordingRuleViewSet(viewsets.ModelViewSet): logger.warning(f"Failed to purge recordings for rule {rule_id}: {err}") +def _stop_dvr_clients(channel_uuid, recording_id=None): + """Stop DVR recording clients for a channel. + + If recording_id is provided, only the client whose User-Agent contains that + recording ID is stopped (safe for simultaneous recordings on the same channel). + If recording_id is None, all Dispatcharr-DVR clients for the channel are stopped + (used by destroy() when deleting a recording whose task_id is unknown). + + Returns the number of DVR clients stopped. + """ + from core.utils import RedisClient + from apps.proxy.ts_proxy.redis_keys import RedisKeys + from apps.proxy.ts_proxy.services.channel_service import ChannelService + + r = RedisClient.get_client() + if not r: + return 0 + client_set_key = RedisKeys.clients(channel_uuid) + client_ids = r.smembers(client_set_key) or [] + stopped = 0 + for raw_id in client_ids: + try: + cid = raw_id.decode("utf-8") if isinstance(raw_id, (bytes, bytearray)) else str(raw_id) + meta_key = RedisKeys.client_metadata(channel_uuid, cid) + ua = r.hget(meta_key, "user_agent") + ua_s = ua.decode("utf-8") if isinstance(ua, (bytes, bytearray)) else (ua or "") + if not (ua_s and "Dispatcharr-DVR" in ua_s): + continue + # When a recording_id is specified, only stop the client for that recording. + # Each run_recording task connects with User-Agent "Dispatcharr-DVR/recording-{id}", + # so we can safely target just this recording without affecting others on the channel. + if recording_id is not None and f"recording-{recording_id}" not in ua_s: + continue + try: + ChannelService.stop_client(channel_uuid, cid) + stopped += 1 + except Exception as inner_e: + logger.debug(f"Failed to stop DVR client {cid} for channel {channel_uuid}: {inner_e}") + except Exception as inner: + logger.debug(f"Error while checking client metadata: {inner}") + # Do not call ChannelService.stop_channel() here. + # Stopping the channel proxy would terminate the source connection which may + # be shared with other recordings on the same channel. The TS proxy server + # already detects when client count reaches zero and tears down the channel + # cleanly on its own (with the configured shutdown delay). + return stopped + + class RecordingViewSet(viewsets.ModelViewSet): queryset = Recording.objects.all() serializer_class = RecordingSerializer @@ -2342,69 +2390,105 @@ class RecordingViewSet(viewsets.ModelViewSet): response["Content-Disposition"] = f"inline; filename=\"{file_name}\"" return response + @action(detail=True, methods=["post"], url_path="stop") + def stop(self, request, pk=None): + """Stop a recording early while retaining the partial content for playback.""" + instance = self.get_object() + + # Mark as stopped in the DB first so run_recording detects it. + # This is the only operation that MUST be synchronous — run_recording reads + # the status field to decide whether the stream disconnection was deliberate. + cp = instance.custom_properties or {} + cp["status"] = "stopped" + cp["stopped_at"] = str(timezone.now()) + instance.custom_properties = cp + instance.save(update_fields=["custom_properties"]) + + # Everything else (DVR client teardown, task revocation, WebSocket notification) + # is moved to a daemon thread so the API returns immediately. Callers were + # seeing 5-15 s delays because _stop_dvr_clients / revoke_task / async_to_sync + # have occasional slow paths (Redis timeouts, Celery control broadcasts, etc.). + channel_uuid = str(instance.channel.uuid) + recording_id = instance.id + task_id = instance.task_id + channel_name = instance.channel.name + + def _background_stop(): + try: + stopped = _stop_dvr_clients(channel_uuid, recording_id=recording_id) + if stopped: + logger.info( + f"Stopped {stopped} DVR client(s) for channel {channel_uuid} (recording stopped early)" + ) + except Exception as e: + logger.debug(f"Unable to stop DVR clients for stopped recording: {e}") + + try: + from apps.channels.signals import revoke_task + revoke_task(task_id) + except Exception as e: + logger.debug(f"Unable to revoke task for stopped recording: {e}") + + try: + from core.utils import send_websocket_update + send_websocket_update('updates', 'update', { + "success": True, + "type": "recording_stopped", + "channel": channel_name, + }) + except Exception: + pass + + # Close the DB connection opened by this thread so it isn't leaked + try: + from django.db import connection as _conn + _conn.close() + except Exception: + pass + + import threading + threading.Thread(target=_background_stop, daemon=True).start() + + return Response({"success": True, "status": "stopped"}) + def destroy(self, request, *args, **kwargs): """Delete the Recording and ensure any active DVR client connection is closed. Also removes the associated file(s) from disk if present. """ instance = self.get_object() - - # Attempt to close the DVR client connection for this channel if active - try: - channel_uuid = str(instance.channel.uuid) - # Lazy imports to avoid module overhead if proxy isn't used - from core.utils import RedisClient - from apps.proxy.ts_proxy.redis_keys import RedisKeys - from apps.proxy.ts_proxy.services.channel_service import ChannelService - - r = RedisClient.get_client() - if r: - client_set_key = RedisKeys.clients(channel_uuid) - client_ids = r.smembers(client_set_key) or [] - stopped = 0 - for raw_id in client_ids: - try: - cid = raw_id.decode("utf-8") if isinstance(raw_id, (bytes, bytearray)) else str(raw_id) - meta_key = RedisKeys.client_metadata(channel_uuid, cid) - ua = r.hget(meta_key, "user_agent") - ua_s = ua.decode("utf-8") if isinstance(ua, (bytes, bytearray)) else (ua or "") - # Identify DVR recording client by its user agent - if ua_s and "Dispatcharr-DVR" in ua_s: - try: - ChannelService.stop_client(channel_uuid, cid) - stopped += 1 - except Exception as inner_e: - logger.debug(f"Failed to stop DVR client {cid} for channel {channel_uuid}: {inner_e}") - except Exception as inner: - logger.debug(f"Error while checking client metadata: {inner}") - if stopped: - logger.info(f"Stopped {stopped} DVR client(s) for channel {channel_uuid} due to recording cancellation") - # If no clients remain after stopping DVR clients, proactively stop the channel - try: - remaining = r.scard(client_set_key) or 0 - except Exception: - remaining = 0 - if remaining == 0: - try: - ChannelService.stop_channel(channel_uuid) - logger.info(f"Stopped channel {channel_uuid} (no clients remain)") - except Exception as sc_e: - logger.debug(f"Unable to stop channel {channel_uuid}: {sc_e}") - except Exception as e: - logger.debug(f"Unable to stop DVR clients for cancelled recording: {e}") + channel_name = instance.channel.name # Capture paths before deletion cp = instance.custom_properties or {} + rec_status = cp.get("status", "") + + # Only stop the DVR client if this recording is actively streaming. + # Stopping for completed/upcoming recordings would kill an unrelated + # in-progress recording on the same channel. + if rec_status == "recording": + try: + channel_uuid = str(instance.channel.uuid) + stopped = _stop_dvr_clients(channel_uuid) + if stopped: + logger.info(f"Stopped {stopped} DVR client(s) for channel {channel_uuid} due to recording cancellation") + except Exception as e: + logger.debug(f"Unable to stop DVR clients for cancelled recording: {e}") file_path = cp.get("file_path") temp_ts_path = cp.get("_temp_file_path") # Perform DB delete first, then try to remove files response = super().destroy(request, *args, **kwargs) - # Notify frontends to refresh recordings + # Notify frontends try: from core.utils import send_websocket_update - send_websocket_update('updates', 'update', {"success": True, "type": "recordings_refreshed"}) + send_websocket_update('updates', 'update', { + "success": True, + "type": "recording_cancelled", + "channel": channel_name, + "was_in_progress": rec_status == "recording", + }) except Exception: pass diff --git a/apps/channels/signals.py b/apps/channels/signals.py index 75ef16dd..00b8fb7c 100644 --- a/apps/channels/signals.py +++ b/apps/channels/signals.py @@ -2,14 +2,15 @@ from django.db.models.signals import m2m_changed, pre_save, post_save, post_delete from django.dispatch import receiver -from django.utils.timezone import now +from django.utils.timezone import now, is_aware, make_aware from celery.result import AsyncResult +from django_celery_beat.models import ClockedSchedule, PeriodicTask from .models import Channel, Stream, ChannelProfile, ChannelProfileMembership, Recording from apps.m3u.models import M3UAccount from apps.epg.tasks import parse_programs_for_tvg_id -import logging, requests, time +import json +import logging from .tasks import run_recording, prefetch_recording_artwork -from django.utils.timezone import now, is_aware, make_aware from datetime import timedelta logger = logging.getLogger(__name__) @@ -85,29 +86,76 @@ def create_profile_memberships(sender, instance, created, **kwargs): for channel in channels ]) +def _dvr_task_name(recording_id): + """Predictable PeriodicTask name for a DVR recording.""" + return f"dvr-recording-{recording_id}" + + def schedule_recording_task(instance, eta=None): - # Use the explicitly-passed (and timezone-aware) eta if provided; - # fall back to instance.start_time only as a last resort. + """Schedule a recording task via ClockedSchedule + one-off PeriodicTask. + + The task is stored in the database and dispatched by Celery Beat at the + scheduled time with no countdown. This avoids the Redis visibility_timeout + redelivery bug that caused duplicate recordings when using apply_async + with long countdowns. See https://github.com/Dispatcharr/Dispatcharr/issues/940 + """ if eta is None: eta = instance.start_time - # Ensure eta is timezone-aware before comparing against now() if eta is not None and not is_aware(eta): eta = make_aware(eta) - # countdown=0 fires immediately (in-progress programs whose start_time was - # clamped to now by the serializer), countdown>0 delays until start_time - # (future programs). Using an integer countdown avoids any timezone - # serialization ambiguity that can occur with an absolute eta datetime. - countdown = max(0, int((eta - now()).total_seconds())) - # Pass recording_id first so task can persist metadata to the correct row - task = run_recording.apply_async( - args=[instance.id, instance.channel_id, str(instance.start_time), str(instance.end_time)], - countdown=countdown, + # Clamp to now so Beat dispatches immediately for past/current start times + if eta <= now(): + eta = now() + + task_args = [ + instance.id, + instance.channel_id, + str(instance.start_time), + str(instance.end_time), + ] + + clocked, _ = ClockedSchedule.objects.get_or_create(clocked_time=eta) + task_name = _dvr_task_name(instance.id) + PeriodicTask.objects.update_or_create( + name=task_name, + defaults={ + "task": "apps.channels.tasks.run_recording", + "clocked": clocked, + "args": json.dumps(task_args), + "one_off": True, + "enabled": True, + "interval": None, + "crontab": None, + "solar": None, + }, ) - return task.id + return task_name + def revoke_task(task_id): - if task_id: + """Cancel a pending recording task. + + task_id is normally a PeriodicTask name (e.g. "dvr-recording-42"). + For backwards compatibility with task_ids stored before this fix + (Celery async-result UUIDs), falls back to AsyncResult.revoke(). + """ + if not task_id: + return + # Primary path: delete the PeriodicTask and clean up its ClockedSchedule + try: + pt = PeriodicTask.objects.get(name=task_id) + old_clocked = pt.clocked + pt.delete() + if old_clocked and not PeriodicTask.objects.filter(clocked=old_clocked).exists(): + old_clocked.delete() + return + except PeriodicTask.DoesNotExist: + pass + # Fallback for legacy Celery task UUIDs + try: AsyncResult(task_id).revoke() + except Exception: + pass @receiver(pre_save, sender=Recording) def revoke_old_task_on_update(sender, instance, **kwargs): @@ -152,11 +200,17 @@ def schedule_task_on_save(sender, instance, created, **kwargs): instance.save(update_fields=['task_id']) else: print("Start time is in the past. Not scheduling.") - # Kick off poster/artwork prefetch to enrich Upcoming cards - try: - prefetch_recording_artwork.apply_async(args=[instance.id], countdown=1) - except Exception as e: - print("Error scheduling artwork prefetch:", e) + # Kick off poster/artwork prefetch to enrich Upcoming cards. + # Skip when the recording is already active or finished — run_recording + # handles its own poster resolution, and scheduling artwork prefetch + # while the task is running causes a race that can overwrite status. + cp = instance.custom_properties or {} + rec_status = cp.get("status", "") + if rec_status not in ("recording", "completed", "stopped", "interrupted"): + try: + prefetch_recording_artwork.apply_async(args=[instance.id], countdown=1) + except Exception as e: + print("Error scheduling artwork prefetch:", e) except Exception as e: import traceback print("Error in post_save signal:", e) diff --git a/apps/channels/tasks.py b/apps/channels/tasks.py index 8d49287b..87abb90d 100755 --- a/apps/channels/tasks.py +++ b/apps/channels/tasks.py @@ -1497,6 +1497,44 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): - Attempts to capture stream stats from TS proxy (codec, resolution, fps, etc.) - Attempts to capture a poster (via program.custom_properties) and store a Logo reference """ + from .models import Recording, Logo + + # --- Idempotency guard (prevents duplicate recordings from task redelivery) --- + try: + rec_check = Recording.objects.filter(id=recording_id).only("custom_properties").first() + if not rec_check: + # Recording was deleted before this task got a chance to run (e.g. user + # cancelled from the Upcoming list, or destroy() ran concurrently). + logger.info( + f"run_recording called for recording {recording_id} but it no longer exists — " + f"skipping (recording was cancelled before task started)." + ) + return + status = (rec_check.custom_properties or {}).get("status", "") + if status in ("recording", "completed", "stopped"): + logger.warning( + f"run_recording called for recording {recording_id} but status " + f"is already '{status}' - skipping duplicate execution." + ) + return + except Exception as e: + logger.debug(f"Idempotency guard check failed (proceeding anyway): {e}") + + # --- Clean up the one-off PeriodicTask that dispatched task --- + try: + from django_celery_beat.models import ClockedSchedule as _CS, PeriodicTask as _PT + task_name = f"dvr-recording-{recording_id}" + try: + pt = _PT.objects.get(name=task_name) + old_clocked = pt.clocked + pt.delete() + if old_clocked and not _PT.objects.filter(clocked=old_clocked).exists(): + old_clocked.delete() + except _PT.DoesNotExist: + pass + except Exception as e: + logger.debug(f"PeriodicTask cleanup failed (non-fatal): {e}") + channel = Channel.objects.get(id=channel_id) start_time = datetime.fromisoformat(start_time_str) @@ -1536,7 +1574,6 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): # Try to resolve the Recording row up front recording_obj = None try: - from .models import Recording, Logo recording_obj = Recording.objects.get(id=recording_id) # Prime custom_properties with file info/status cp = recording_obj.custom_properties or {} @@ -1822,7 +1859,7 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): with requests.get( test_url, headers={ - 'User-Agent': 'Dispatcharr-DVR', + 'User-Agent': f'Dispatcharr-DVR/recording-{recording_id}', }, stream=True, timeout=(10, 15), @@ -1878,6 +1915,38 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): except Exception as e: last_error = str(e) logger.warning(f"DVR: attempt failed for base {base}: {e}") + # If there was a working stream that was interrupted (e.g. recording + # cancelled via stop_client), do not retry on another base — that would + # re-establish the DVR connection and make the recording re-appear. + if bytes_written > 0: + # Check if this was a deliberate "stop" vs an unexpected interruption + try: + rec_check = Recording.objects.filter(id=recording_id).only("custom_properties").first() + if rec_check and (rec_check.custom_properties or {}).get("status") == "stopped": + # Graceful stop — not an error, just an early end + interrupted = False + logger.info(f"Recording {recording_id} was stopped by user — ending stream.") + else: + interrupted = True + interrupted_reason = f"stream_interrupted: {e}" + except Exception: + interrupted = True + interrupted_reason = f"stream_interrupted: {e}" + break + # Also bail if the Recording was deleted (user cancelled before data arrived) + rec_exists = Recording.objects.filter(id=recording_id).exists() + if not rec_exists: + interrupted = True + interrupted_reason = "recording_deleted_during_connect" + break + # Check for "stopped" status even before data arrived + try: + rec_check = Recording.objects.filter(id=recording_id).only("custom_properties").first() + if rec_check and (rec_check.custom_properties or {}).get("status") == "stopped": + interrupted = False + break + except Exception: + pass if chosen_base is None and bytes_written == 0: interrupted = True @@ -1892,39 +1961,48 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): # We cannot compute accurate elapsed here; fine to leave as is pass - # If no bytes were written at all, mark detail + # If no bytes were written at all, check whether this was a deliberate stop or a + # genuine failure. The exception handler above already sets interrupted=False when + # it detects "stopped" status, but do not override that decision here. if bytes_written == 0 and not interrupted: - interrupted = True - interrupted_reason = f"no_stream_data: {last_error or 'unknown'}" - - # Update DB status immediately so the UI reflects the change on the event below + _deliberately_stopped = False try: - if recording_obj is None: - from .models import Recording - recording_obj = Recording.objects.get(id=recording_id) - cp_now = recording_obj.custom_properties or {} - cp_now.update({ - "status": "interrupted" if interrupted else "completed", - "ended_at": str(datetime.now()), - "file_name": filename or cp_now.get("file_name"), - "file_path": final_path or cp_now.get("file_path"), - }) - if interrupted and interrupted_reason: - cp_now["interrupted_reason"] = interrupted_reason - recording_obj.custom_properties = cp_now - recording_obj.save(update_fields=["custom_properties"]) - except Exception as e: - logger.debug(f"Failed to update immediate recording status: {e}") + _rc = Recording.objects.filter(id=recording_id).only("custom_properties").first() + if _rc and (_rc.custom_properties or {}).get("status") == "stopped": + _deliberately_stopped = True + except Exception: + pass - async_to_sync(channel_layer.group_send)( - "updates", - { - "type": "update", - "data": {"success": True, "type": "recording_ended", "channel": channel.name} - }, - ) - # After the loop, the file and response are closed automatically. - logger.info(f"Finished recording for channel {channel.name}") + if not _deliberately_stopped: + interrupted = True + interrupted_reason = f"no_stream_data: {last_error or 'unknown'}" + + # Update DB status immediately so the UI reflects the change on the event below + try: + if recording_obj is None: + recording_obj = Recording.objects.get(id=recording_id) + cp_now = recording_obj.custom_properties or {} + cp_now.update({ + "status": "interrupted", + "ended_at": str(datetime.now()), + "file_name": filename or cp_now.get("file_name"), + "file_path": final_path or cp_now.get("file_path"), + "interrupted_reason": interrupted_reason, + }) + recording_obj.custom_properties = cp_now + recording_obj.save(update_fields=["custom_properties"]) + except Exception as e: + logger.debug(f"Failed to update immediate recording status: {e}") + + async_to_sync(channel_layer.group_send)( + "updates", + { + "type": "update", + "data": {"success": True, "type": "recording_ended", "channel": channel.name} + }, + ) + # After the loop, the file and response are closed automatically. + logger.info(f"Finished recording for channel {channel.name}") # Log system event for recording end try: @@ -1940,6 +2018,20 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): except Exception as e: logger.error(f"Could not log recording end event: {e}") + # If the Recording was deleted (cancelled by user), skip post-processing + recording_cancelled = not Recording.objects.filter(id=recording_id).exists() + if recording_cancelled: + logger.info(f"Recording {recording_id} was cancelled — skipping remux and metadata.") + # Clean up any remaining temp or stub output files + for _cleanup_path in [temp_ts_path, final_path]: + try: + if _cleanup_path and os.path.exists(_cleanup_path): + os.remove(_cleanup_path) + logger.info(f"Cleaned up cancelled recording artifact: {_cleanup_path}") + except Exception: + pass + return + # Remux TS to MKV container remux_success = False try: @@ -2039,19 +2131,25 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): # Persist final metadata to Recording (status, ended_at, and stream stats if available) try: if recording_obj is None: - from .models import Recording recording_obj = Recording.objects.get(id=recording_id) + # Re-read from DB to get the latest status (stop endpoint may have set it) + recording_obj.refresh_from_db() cp = recording_obj.custom_properties or {} - cp.update({ - "ended_at": str(datetime.now()), - }) - if interrupted: + cp["ended_at"] = str(datetime.now()) + + # Determine final status. + # A deliberate user stop ("stopped" in DB) always wins over any interrupted flag — + # this handles races where the stream connection broke milliseconds before or + # after the stop endpoint committed its DB save. + db_status_now = cp.get("status", "") + if db_status_now == "stopped" or not interrupted: + cp["status"] = "completed" + cp.pop("interrupted_reason", None) + else: cp["status"] = "interrupted" if interrupted_reason: cp["interrupted_reason"] = interrupted_reason - else: - cp["status"] = "completed" cp["bytes_written"] = bytes_written cp["remux_success"] = remux_success @@ -2112,8 +2210,29 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): # Removed: local thumbnail generation. We rely on EPG/VOD/TMDB/OMDb/keyless providers only. + # Final cancellation guard: destroy() may have deleted the record while + # remuxing. If it's gone now, skip saving "interrupted" status and + # skip the notification — destroy() already sent recording_cancelled. + if not Recording.objects.filter(id=recording_id).exists(): + logger.info( + f"Recording {recording_id} was deleted during post-processing — skipping final save." + ) + return + recording_obj.custom_properties = cp recording_obj.save(update_fields=["custom_properties"]) + + # Notify frontends so the UI refreshes immediately (e.g. "Stopped" → "Completed") + try: + async_to_sync(channel_layer.group_send)( + "updates", + { + "type": "update", + "data": {"success": True, "type": "recording_ended", "channel": channel.name}, + }, + ) + except Exception: + pass except Exception as e: logger.debug(f"Unable to finalize Recording metadata: {e}") @@ -2154,6 +2273,20 @@ def recover_recordings_on_startup(): for rec in active: try: cp = rec.custom_properties or {} + current_status = cp.get("status", "") + + # Skip recordings that are already in a terminal or active state. + # "completed" / "stopped" — user stopped or it finished normally; do NOT + # overwrite the status and re-schedule (that would cause the + # Interrupted → In-Progress → Previously-Recorded ghost cycle). + # "recording" — a worker is already streaming; leave it alone. + if current_status in ("completed", "stopped", "recording"): + logger.info( + f"recover_recordings_on_startup: skipping recording {rec.id} " + f"(status={current_status!r}, already in terminal/active state)." + ) + continue + # Mark interrupted due to restart; will flip to 'recording' when task starts cp["status"] = "interrupted" cp["interrupted_reason"] = "server_restarted" @@ -2167,11 +2300,20 @@ def recover_recordings_on_startup(): except Exception as e: logger.warning(f"Failed to resume recording {rec.id}: {e}") - # Ensure future recordings are scheduled + # Ensure future recordings are scheduled. + # With ClockedSchedule, PeriodicTasks survive restarts in the DB. + # Only recreate if the PeriodicTask is missing (safety net). upcoming = Recording.objects.filter(start_time__gt=now, end_time__gt=now) for rec in upcoming: try: - # Schedule task at start_time + from django_celery_beat.models import PeriodicTask as _PT + task_name = f"dvr-recording-{rec.id}" + if _PT.objects.filter(name=task_name).exists(): + if rec.task_id != task_name: + rec.task_id = task_name + rec.save(update_fields=["task_id"]) + continue + # PeriodicTask missing - recreate it task_id = schedule_recording_task(rec) if task_id: rec.task_id = task_id @@ -2553,6 +2695,13 @@ def prefetch_recording_artwork(recording_id): from .models import Recording rec = Recording.objects.get(id=recording_id) cp = rec.custom_properties or {} + + # Bail out if the recording is already active or finished — run_recording + # handles poster resolution itself, and saving here can race with status updates. + current_status = cp.get("status", "") + if current_status in ("recording", "completed", "stopped", "interrupted"): + return "skipped: status is " + current_status + program = cp.get("program") or {} poster_logo_id, poster_url = _resolve_poster_for_program(rec.channel.name, program) updated = False @@ -2593,7 +2742,15 @@ def prefetch_recording_artwork(recording_id): pass if updated: - rec.custom_properties = cp + # Re-read from DB to avoid overwriting status changes made by + # the stop endpoint or run_recording's final metadata save. + rec.refresh_from_db() + fresh_cp = rec.custom_properties or {} + for key in ("poster_logo_id", "poster_url", "rating", "rating_system", + "season", "episode", "onscreen_episode"): + if key in cp: + fresh_cp[key] = cp[key] + rec.custom_properties = fresh_cp rec.save(update_fields=["custom_properties"]) try: from core.utils import send_websocket_update diff --git a/apps/channels/tests/test_recording_scheduling.py b/apps/channels/tests/test_recording_scheduling.py new file mode 100644 index 00000000..56bdbe87 --- /dev/null +++ b/apps/channels/tests/test_recording_scheduling.py @@ -0,0 +1,547 @@ +"""Tests for DVR recording scheduling with ClockedSchedule. + + Prefers Celery ClockedScheduled over Redis time due to 3600s limitation on scheduled tasks; + scheduled tasks beyond 3600s cause Redis time to hand process between workers and duplicate jobs +""" +from datetime import timedelta +from unittest.mock import patch, MagicMock + +from django.test import TestCase +from django.utils import timezone +from django_celery_beat.models import ClockedSchedule, PeriodicTask + +from apps.channels.models import Channel, Recording +from apps.channels.signals import ( + schedule_recording_task, + revoke_task, + _dvr_task_name, +) + + +class ScheduleRecordingTaskTests(TestCase): + """Tests for schedule_recording_task().""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + def tearDown(self): + PeriodicTask.objects.filter(name__startswith="dvr-recording-").delete() + ClockedSchedule.objects.all().delete() + + @patch("apps.channels.signals.run_recording") + def test_future_recording_creates_periodic_task(self, mock_run_recording): + """Recordings in the future create a ClockedSchedule + PeriodicTask.""" + future_time = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future_time, + end_time=future_time + timedelta(hours=1), + ) + + task_id = schedule_recording_task(rec, eta=future_time) + + expected_name = f"dvr-recording-{rec.id}" + self.assertEqual(task_id, expected_name) + + pt = PeriodicTask.objects.get(name=expected_name) + self.assertTrue(pt.one_off) + self.assertTrue(pt.enabled) + self.assertEqual(pt.task, "apps.channels.tasks.run_recording") + self.assertIsNotNone(pt.clocked) + + # apply_async should not have been called + mock_run_recording.apply_async.assert_not_called() + + @patch("apps.channels.signals.run_recording") + def test_immediate_recording_creates_periodic_task(self, mock_run_recording): + """Recordings starting now also use ClockedSchedule for consistency.""" + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now, + end_time=now + timedelta(hours=1), + ) + + task_id = schedule_recording_task(rec, eta=now) + + expected_name = f"dvr-recording-{rec.id}" + self.assertEqual(task_id, expected_name) + self.assertTrue(PeriodicTask.objects.filter(name=expected_name).exists()) + + @patch("apps.channels.signals.run_recording") + def test_past_start_time_clamps_to_now(self, mock_run_recording): + """Recordings with past start_time get clamped to now.""" + past_time = timezone.now() - timedelta(minutes=5) + rec = Recording.objects.create( + channel=self.channel, + start_time=past_time, + end_time=timezone.now() + timedelta(hours=1), + ) + + task_id = schedule_recording_task(rec, eta=past_time) + + expected_name = f"dvr-recording-{rec.id}" + self.assertEqual(task_id, expected_name) + pt = PeriodicTask.objects.get(name=expected_name) + # Clocked time should be >= now + self.assertGreaterEqual(pt.clocked.clocked_time, past_time) + + @patch("apps.channels.signals.run_recording") + def test_reschedule_updates_existing_periodic_task(self, mock_run_recording): + """Calling schedule_recording_task twice updates the existing PeriodicTask.""" + future_time = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future_time, + end_time=future_time + timedelta(hours=1), + ) + + schedule_recording_task(rec, eta=future_time) + + # Reschedule with a different time + new_eta = future_time + timedelta(hours=1) + schedule_recording_task(rec, eta=new_eta) + + # Should still be exactly one PeriodicTask + task_name = f"dvr-recording-{rec.id}" + self.assertEqual(PeriodicTask.objects.filter(name=task_name).count(), 1) + + @patch("apps.channels.signals.run_recording") + def test_naive_eta_is_made_aware(self, mock_run_recording): + """A naive (timezone-unaware) eta is made timezone-aware.""" + from datetime import datetime + naive_eta = datetime(2030, 6, 15, 14, 0, 0) + rec = Recording.objects.create( + channel=self.channel, + start_time=timezone.now() + timedelta(hours=1), + end_time=timezone.now() + timedelta(hours=2), + ) + + task_id = schedule_recording_task(rec, eta=naive_eta) + + expected_name = f"dvr-recording-{rec.id}" + self.assertEqual(task_id, expected_name) + pt = PeriodicTask.objects.get(name=expected_name) + self.assertTrue(timezone.is_aware(pt.clocked.clocked_time)) + + +class RevokeTaskTests(TestCase): + """Tests for revoke_task().""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + def tearDown(self): + PeriodicTask.objects.filter(name__startswith="dvr-recording-").delete() + ClockedSchedule.objects.all().delete() + + def test_revoke_deletes_periodic_task_and_clocked_schedule(self): + """revoke_task deletes the PeriodicTask and orphaned ClockedSchedule.""" + eta = timezone.now() + timedelta(hours=5) + clocked = ClockedSchedule.objects.create(clocked_time=eta) + PeriodicTask.objects.create( + name="dvr-recording-10", + task="apps.channels.tasks.run_recording", + clocked=clocked, + one_off=True, + enabled=True, + ) + + revoke_task("dvr-recording-10") + + self.assertFalse(PeriodicTask.objects.filter(name="dvr-recording-10").exists()) + self.assertFalse(ClockedSchedule.objects.filter(id=clocked.id).exists()) + + def test_revoke_keeps_shared_clocked_schedule(self): + """ClockedSchedule is kept if another PeriodicTask still references it.""" + eta = timezone.now() + timedelta(hours=5) + clocked = ClockedSchedule.objects.create(clocked_time=eta) + PeriodicTask.objects.create( + name="dvr-recording-10", + task="apps.channels.tasks.run_recording", + clocked=clocked, + one_off=True, + ) + PeriodicTask.objects.create( + name="dvr-recording-11", + task="apps.channels.tasks.run_recording", + clocked=clocked, + one_off=True, + ) + + revoke_task("dvr-recording-10") + + self.assertFalse(PeriodicTask.objects.filter(name="dvr-recording-10").exists()) + self.assertTrue(ClockedSchedule.objects.filter(id=clocked.id).exists()) + + @patch("apps.channels.signals.AsyncResult") + def test_revoke_falls_back_to_async_result_for_legacy_ids(self, mock_async_result): + """revoke_task falls back to AsyncResult.revoke() for old-style UUIDs.""" + revoke_task("550e8400-e29b-41d4-a716-446655440000") + + mock_async_result.assert_called_once_with("550e8400-e29b-41d4-a716-446655440000") + mock_async_result.return_value.revoke.assert_called_once() + + def test_revoke_none_is_noop(self): + """revoke_task(None) does nothing.""" + revoke_task(None) # Should not raise + + def test_revoke_empty_string_is_noop(self): + """revoke_task('') does nothing.""" + revoke_task("") # Should not raise + + +class DvrTaskNameTests(TestCase): + """Tests for the naming convention helper.""" + + def test_task_name_format(self): + self.assertEqual(_dvr_task_name(42), "dvr-recording-42") + + def test_task_name_fits_in_charfield(self): + name = _dvr_task_name(999999999) + self.assertLessEqual(len(name), 255) + + +class SignalIntegrationTests(TestCase): + """Integration tests for the post_save / post_delete signal handlers.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + def tearDown(self): + PeriodicTask.objects.filter(name__startswith="dvr-recording-").delete() + ClockedSchedule.objects.all().delete() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_post_save_creates_periodic_task_for_future_recording(self, mock_artwork): + """Saving a future Recording creates a PeriodicTask via post_save signal.""" + mock_artwork.apply_async.return_value = MagicMock() + + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + ) + + rec.refresh_from_db() + task_name = f"dvr-recording-{rec.id}" + self.assertEqual(rec.task_id, task_name) + self.assertTrue(PeriodicTask.objects.filter(name=task_name).exists()) + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_post_delete_removes_periodic_task(self, mock_artwork): + """Deleting a Recording removes its PeriodicTask.""" + mock_artwork.apply_async.return_value = MagicMock() + + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + ) + + rec.refresh_from_db() + task_name = rec.task_id + self.assertTrue(PeriodicTask.objects.filter(name=task_name).exists()) + + rec.delete() + self.assertFalse(PeriodicTask.objects.filter(name=task_name).exists()) + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_bulk_delete_cleans_up_all_periodic_tasks(self, mock_artwork): + """Bulk deleting recordings cleans up all their PeriodicTasks.""" + mock_artwork.apply_async.return_value = MagicMock() + + future = timezone.now() + timedelta(hours=2) + rec_ids = [] + for i in range(5): + rec = Recording.objects.create( + channel=self.channel, + start_time=future + timedelta(hours=i), + end_time=future + timedelta(hours=i + 1), + ) + rec_ids.append(rec.id) + + for rid in rec_ids: + self.assertTrue( + PeriodicTask.objects.filter(name=f"dvr-recording-{rid}").exists() + ) + + Recording.objects.filter(channel=self.channel).delete() + + self.assertEqual( + PeriodicTask.objects.filter(name__startswith="dvr-recording-").count(), 0 + ) + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_pre_save_revokes_on_time_change(self, mock_artwork): + """Changing a recording's start_time revokes the old task and creates a new one.""" + mock_artwork.apply_async.return_value = MagicMock() + + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + ) + + rec.refresh_from_db() + old_task_name = rec.task_id + self.assertTrue(PeriodicTask.objects.filter(name=old_task_name).exists()) + + # Change the start time — pre_save clears task_id, post_save reschedules + new_future = future + timedelta(hours=3) + rec.start_time = new_future + rec.end_time = new_future + timedelta(hours=1) + rec.save() + + rec.refresh_from_db() + # Old PeriodicTask should be deleted; new one should exist + self.assertIsNotNone(rec.task_id) + self.assertTrue( + PeriodicTask.objects.filter(name=f"dvr-recording-{rec.id}").exists() + ) + + +class IdempotencyGuardTests(TestCase): + """Tests for the idempotency guard in run_recording().""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + @patch("apps.channels.tasks.get_channel_layer") + def test_skips_if_already_recording(self, mock_layer): + """run_recording returns early if status is already 'recording'.""" + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now, + end_time=now + timedelta(hours=1), + custom_properties={"status": "recording", "started_at": str(now)}, + ) + + from apps.channels.tasks import run_recording as run_rec_task + result = run_rec_task(rec.id, self.channel.id, str(now), str(now + timedelta(hours=1))) + + self.assertIsNone(result) + # get_channel_layer should not have been called (returned before) + mock_layer.assert_not_called() + + @patch("apps.channels.tasks.get_channel_layer") + def test_skips_if_already_completed(self, mock_layer): + """run_recording returns early if status is already 'completed'.""" + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(hours=2), + end_time=now - timedelta(hours=1), + custom_properties={"status": "completed"}, + ) + + from apps.channels.tasks import run_recording as run_rec_task + result = run_rec_task(rec.id, self.channel.id, str(rec.start_time), str(rec.end_time)) + + self.assertIsNone(result) + mock_layer.assert_not_called() + + @patch("apps.channels.tasks.get_channel_layer") + def test_skips_if_already_stopped(self, mock_layer): + """run_recording returns early if status is already 'stopped' (user stopped it early).""" + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(hours=1), + end_time=now + timedelta(hours=1), + custom_properties={"status": "stopped", "stopped_at": str(now)}, + ) + + from apps.channels.tasks import run_recording as run_rec_task + result = run_rec_task(rec.id, self.channel.id, str(rec.start_time), str(rec.end_time)) + + self.assertIsNone(result) + mock_layer.assert_not_called() + + +class ArtworkPrefetchSignalGuardTests(TestCase): + """Tests that the post_save signal does not schedule artwork prefetch when + the recording is in an active or terminal state.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + def tearDown(self): + PeriodicTask.objects.filter(name__startswith="dvr-recording-").delete() + ClockedSchedule.objects.all().delete() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_artwork_prefetch_not_scheduled_when_status_recording(self, mock_artwork): + """post_save must NOT schedule artwork prefetch when status='recording' + to prevent a race that overwrites the running task's status updates.""" + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={"status": "recording"}, + ) + + # Simulate a save that run_recording itself might do mid-recording + rec.custom_properties = {"status": "recording", "file_path": "/data/recordings/test.mkv"} + rec.save(update_fields=["custom_properties"]) + + # apply_async was not called for the "recording" save + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_artwork_prefetch_not_scheduled_when_status_completed(self, mock_artwork): + """post_save must NOT schedule artwork prefetch when status='completed'.""" + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={"status": "completed"}, + ) + + rec.custom_properties = {"status": "completed"} + rec.save(update_fields=["custom_properties"]) + + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_artwork_prefetch_not_scheduled_when_status_stopped(self, mock_artwork): + """post_save must NOT schedule artwork prefetch when status='stopped'.""" + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={"status": "stopped"}, + ) + + rec.custom_properties = {"status": "stopped"} + rec.save(update_fields=["custom_properties"]) + + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_artwork_prefetch_scheduled_for_new_upcoming_recording(self, mock_artwork): + """post_save SHOULD schedule artwork prefetch for a newly created upcoming recording.""" + mock_artwork.apply_async.return_value = MagicMock() + future = timezone.now() + timedelta(hours=2) + Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={}, # no status yet — should trigger prefetch + ) + + self.assertTrue(mock_artwork.apply_async.called) + + +class DestroyDvrClientIsolationTests(TestCase): + """Tests that deleting a recording only stops DVR clients when the + recording is actively streaming — never for completed/upcoming recordings + that could share a channel with an unrelated in-progress recording.""" + + def setUp(self): + from django.contrib.auth import get_user_model + from rest_framework.test import APIRequestFactory, force_authenticate + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + User = get_user_model() + self.user = User.objects.create_user( + username="dvr_test_admin", password="pass", + user_level=User.UserLevel.ADMIN, + ) + self.factory = APIRequestFactory() + self.force_authenticate = force_authenticate + + def _delete_recording(self, rec): + from apps.channels.api_views import RecordingViewSet + request = self.factory.delete(f"/api/channels/recordings/{rec.id}/") + self.force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"delete": "destroy"}) + return view(request, pk=rec.id) + + @patch("apps.channels.api_views._stop_dvr_clients") + def test_destroy_completed_recording_does_not_stop_dvr_clients(self, mock_stop): + """Deleting a completed recording must NOT call _stop_dvr_clients.""" + rec = Recording.objects.create( + channel=self.channel, + start_time=timezone.now() - timedelta(hours=2), + end_time=timezone.now() - timedelta(hours=1), + custom_properties={"status": "completed", "file_path": "/data/recordings/test.mkv"}, + ) + self._delete_recording(rec) + mock_stop.assert_not_called() + + @patch("apps.channels.api_views._stop_dvr_clients") + def test_destroy_upcoming_recording_does_not_stop_dvr_clients(self, mock_stop): + """Deleting an upcoming (scheduled) recording must NOT call _stop_dvr_clients.""" + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={}, + ) + self._delete_recording(rec) + mock_stop.assert_not_called() + + @patch("apps.channels.api_views._stop_dvr_clients") + def test_destroy_active_recording_does_stop_dvr_clients(self, mock_stop): + """Deleting an in-progress recording MUST call _stop_dvr_clients.""" + mock_stop.return_value = 1 + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(minutes=5), + end_time=now + timedelta(hours=1), + custom_properties={"status": "recording"}, + ) + self._delete_recording(rec) + mock_stop.assert_called_once_with(str(self.channel.uuid)) + + +class PeriodicTaskCleanupOnExecutionTests(TestCase): + """Tests for PeriodicTask cleanup when run_recording starts.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=1, name="Test Channel") + + def tearDown(self): + PeriodicTask.objects.filter(name__startswith="dvr-recording-").delete() + ClockedSchedule.objects.all().delete() + + @patch("apps.channels.signals.prefetch_recording_artwork") + @patch("apps.channels.tasks.get_channel_layer") + def test_periodic_task_cleaned_up_on_execution(self, mock_layer, mock_artwork): + """When run_recording executes, it deletes its own PeriodicTask.""" + mock_layer.return_value = MagicMock() + mock_artwork.apply_async.return_value = MagicMock() + + future = timezone.now() + timedelta(hours=2) + rec = Recording.objects.create( + channel=self.channel, + start_time=future, + end_time=future + timedelta(hours=1), + custom_properties={}, + ) + + # post_save signal should have created the PeriodicTask + task_name = f"dvr-recording-{rec.id}" + self.assertTrue(PeriodicTask.objects.filter(name=task_name).exists()) + pt = PeriodicTask.objects.get(name=task_name) + clocked_id = pt.clocked_id + + from apps.channels.tasks import run_recording as run_rec_task + # This will proceed past guards, clean up the PeriodicTask, then + # eventually fail on the actual stream connection (expected) + try: + run_rec_task(rec.id, self.channel.id, str(future), str(future + timedelta(hours=1))) + except Exception: + pass + + self.assertFalse(PeriodicTask.objects.filter(name=task_name).exists()) + self.assertFalse(ClockedSchedule.objects.filter(id=clocked_id).exists()) diff --git a/frontend/src/WebSocket.jsx b/frontend/src/WebSocket.jsx index c18a7635..f595b747 100644 --- a/frontend/src/WebSocket.jsx +++ b/frontend/src/WebSocket.jsx @@ -547,6 +547,34 @@ export const WebsocketProvider = ({ children }) => { } break; + case 'recording_stopped': + notifications.show({ + title: 'Recording stopped', + message: `Recording stopped early for ${parsedEvent.data.channel || 'channel'}. Partial content has been saved.`, + color: 'yellow', + }); + try { + await useChannelsStore.getState().fetchRecordings(); + } catch (e) { + console.warn('Failed to refresh recordings on stop:', e); + } + break; + + case 'recording_cancelled': + notifications.show({ + title: parsedEvent.data.was_in_progress ? 'Recording cancelled' : 'Recording deleted', + message: parsedEvent.data.was_in_progress + ? 'Recording cancelled and content removed.' + : 'Recording deleted.', + color: 'red', + }); + try { + await useChannelsStore.getState().fetchRecordings(); + } catch (e) { + console.warn('Failed to refresh recordings on cancel:', e); + } + break; + case 'epg_fetch_error': notifications.show({ title: 'EPG Source Error', diff --git a/frontend/src/api.js b/frontend/src/api.js index 10a836ae..4fbf1018 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -2662,6 +2662,17 @@ export default class API { } } + static async stopRecording(id) { + try { + await request(`${host}/api/channels/recordings/${id}/stop/`, { + method: 'POST', + }); + } catch (e) { + errorNotification(`Failed to stop recording ${id}`, e); + throw e; + } + } + static async runComskip(recordingId) { try { const resp = await request( diff --git a/frontend/src/components/cards/RecordingCard.jsx b/frontend/src/components/cards/RecordingCard.jsx index e1dbf020..2f428768 100644 --- a/frontend/src/components/cards/RecordingCard.jsx +++ b/frontend/src/components/cards/RecordingCard.jsx @@ -22,7 +22,7 @@ import { Text, Tooltip, } from '@mantine/core'; -import { AlertTriangle, SquareX } from 'lucide-react'; +import { AlertTriangle, Square, SquareX } from 'lucide-react'; import RecordingSynopsis from '../RecordingSynopsis'; import { deleteRecordingById, @@ -34,6 +34,7 @@ import { getShowVideoUrl, removeRecording, runComSkip, + stopRecordingById, } from './../../utils/cards/RecordingCardUtils.js'; const RecordingCard = ({ @@ -73,7 +74,7 @@ const RecordingCard = ({ const status = customProps.status; const isTimeActive = now.isAfter(start) && now.isBefore(end); const isInterrupted = status === 'interrupted'; - const isInProgress = isTimeActive; // Show as recording by time, regardless of status glitches + const isInProgress = isTimeActive && !isInterrupted && status !== 'completed' && status !== 'stopped'; const isUpcoming = now.isBefore(start); const isSeriesGroup = Boolean( recording._group_count && recording._group_count > 1 @@ -117,10 +118,18 @@ const RecordingCard = ({ } }; - // Cancel handling for series groups + // Stop / Cancel / Delete state and handlers const [cancelOpen, setCancelOpen] = React.useState(false); + const [stopConfirmOpen, setStopConfirmOpen] = React.useState(false); + const [deleteConfirmOpen, setDeleteConfirmOpen] = React.useState(false); const [busy, setBusy] = React.useState(false); - const handleCancelClick = (e) => { + + const handleStopClick = (e) => { + e.stopPropagation(); + setStopConfirmOpen(true); + }; + + const handleDeleteClick = (e) => { e.stopPropagation(); if (isRecurringRule) { onOpenRecurring?.(recording, true); @@ -129,7 +138,30 @@ const RecordingCard = ({ if (isSeriesGroup) { setCancelOpen(true); } else { + setDeleteConfirmOpen(true); + } + }; + + const confirmStop = async () => { + try { + setBusy(true); + await stopRecordingById(recording.id); + } catch (error) { + console.error('Failed to stop recording', error); + } finally { + setBusy(false); + setStopConfirmOpen(false); + try { await fetchRecordings(); } catch {} + } + }; + + const confirmDelete = async () => { + try { + setBusy(true); removeRecording(recording.id); + } finally { + setBusy(false); + setDeleteConfirmOpen(false); } }; @@ -279,18 +311,30 @@ const RecordingCard = ({ -
- + + {isInProgress && ( + + e.stopPropagation()} + onClick={handleStopClick} + > + + + + )} + e.stopPropagation()} - onClick={handleCancelClick} + onClick={handleDeleteClick} > -
+ @@ -378,11 +422,74 @@ const RecordingCard = ({ )} ); - if (!isSeriesGroup) return MainCard; + + // Confirmation modals for stop and cancel/delete + const ConfirmModals = ( + <> + setStopConfirmOpen(false)} + title="Stop Recording" + centered + size="md" + zIndex={9999} + > + + + The recording will be stopped early. The portion already recorded + will be saved and available for playback. + + + + + + + + + setDeleteConfirmOpen(false)} + title={isInProgress ? 'Cancel Recording' : isUpcoming ? 'Cancel Recording' : 'Delete Recording'} + centered + size="md" + zIndex={9999} + > + + + {isInProgress + ? 'The recording will be cancelled and all recorded content will be permanently deleted.' + : isUpcoming + ? 'This scheduled recording will be cancelled.' + : 'This recording and all associated files will be permanently deleted.'} + + + + + + + + + ); + + if (!isSeriesGroup) return ( + <> + {ConfirmModals} + {MainCard} + + ); // Stacked look for series groups: render two shadow layers behind the main card return ( + {ConfirmModals} setCancelOpen(false)} diff --git a/frontend/src/utils/cards/RecordingCardUtils.js b/frontend/src/utils/cards/RecordingCardUtils.js index 6232064b..60dd2cd5 100644 --- a/frontend/src/utils/cards/RecordingCardUtils.js +++ b/frontend/src/utils/cards/RecordingCardUtils.js @@ -51,6 +51,10 @@ export const deleteRecordingById = async (recordingId) => { await API.deleteRecording(recordingId); }; +export const stopRecordingById = async (recordingId) => { + await API.stopRecording(recordingId); +}; + export const deleteSeriesAndRule = async (seriesInfo) => { const { tvg_id, title } = seriesInfo; if (tvg_id) { diff --git a/frontend/src/utils/pages/DVRUtils.js b/frontend/src/utils/pages/DVRUtils.js index bcb200b2..fb80dd15 100644 --- a/frontend/src/utils/pages/DVRUtils.js +++ b/frontend/src/utils/pages/DVRUtils.js @@ -32,7 +32,7 @@ const dedupeById = (list, toUserTime, completed, now, inProgress, upcoming) => { const e = toUserTime(rec.end_time); const status = rec.custom_properties?.status; - if (status === 'interrupted' || status === 'completed') { + if (status === 'interrupted' || status === 'completed' || status === 'stopped') { completed.push(rec); } else { if (now.isAfter(s) && now.isBefore(e)) inProgress.push(rec); From f28530a47b23ab2f9a6babbf3949f999cca997c4 Mon Sep 17 00:00:00 2001 From: None Date: Sun, 1 Mar 2026 17:01:16 -0600 Subject: [PATCH 36/58] fix(dvr): add recording extend, fix cross-recording interference, artwork race, and keepalive gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File changes: Backend apps/channels/api_views.py — Added extend() action that uses queryset .update() to bypass signals, letting the running task pick up the new end_time via polling. Moved WS recording_stopped notification to synchronous (before HTTP response). Refactored destroy() to delete DB row first, send WS event immediately, defer cleanup to background thread. apps/channels/signals.py — Added pre_save guard to skip task revocation when status is "recording". Added post_save reentrancy guard to skip artwork prefetch for internal field-only saves (custom_properties, task_id, end_time). apps/channels/tasks.py — Added 2-second polling loop in streaming that checks for stop, deletion, and extended end_time. Added refresh_from_db() + merge before metadata save to prevent overwriting concurrent artwork prefetch. Replaced inline PeriodicTask cleanup with revoke_task()/_dvr_task_name(). Consolidated redundant DB queries in error path. Fixed TOCTOU os.path.exists + os.remove. Preserved "stopped" status in final-status logic instead of overwriting with "completed". core/utils.py — Made send_websocket_update gevent-safe by detecting monkey-patching and offloading async_to_sync to gevent's native threadpool, fixing the event loop deadlock that caused cross-recording interference. apps/proxy/ts_proxy/client_manager.py — Moved _trigger_stats_update to a background thread so remove_client() returns immediately. apps/proxy/ts_proxy/stream_generator.py — Added Redis-based health check for non-owner workers in _should_send_keepalive(), so keepalives fire correctly even when stream_manager is None. Frontend frontend/src/WebSocket.jsx — Added 400ms debounced scheduleRecordingFetch() replacing all inline fetchRecordings() calls. Added recording_extended and recording_updated event handlers. recording_cancelled now does surgical removeRecording() by ID when available. frontend/src/api.js — Added extendRecording(id, extraMinutes). frontend/src/components/cards/RecordingCard.jsx — Added Extend menu (+15m/+30m/+1h) for in-progress recordings. Widened Comskip button to stopped/interrupted statuses. frontend/src/components/forms/ProgramRecordingModal.jsx — Removed manual fetchRecordings() calls, relying on WS-driven debounced refresh. frontend/src/components/forms/RecordingDetailsModal.jsx — Added stopped to playable statuses. Widened Comskip button. Removed manual fetchRecordings(). frontend/src/components/forms/RecurringRuleModal.jsx — Removed all manual fetchRecordings() calls, relying on WS events. frontend/src/pages/Guide.jsx — Removed isLoading subscription that caused loading flash on debounced refresh. Removed manual fetchRecordings() after saving a series rule. frontend/src/store/channels.jsx — Stripped isLoading/error state from fetchRecordings to prevent full-page loading indicators. Added no-op guards in removeRecording. frontend/src/utils/cards/RecordingCardUtils.js — Added extendRecordingById wrapper. Tests apps/channels/tests/test_recording_extend.py (new) — 14 tests: extend endpoint validation, stacked extensions, signal bypass, pre_save guard behavior. apps/channels/tests/test_recording_stop_cancel.py (new) — 18 tests: stop endpoint, DVR client teardown, cancel was_in_progress flag, run_recording race guards, signal reentrancy guards. apps/channels/tests/test_ts_proxy_keepalive.py (new) — 21 tests: keepalive owner/non-owner logic, stats update error handling, client removal non-blocking, timing invariants. apps/channels/tests/test_recording_scheduling.py — Updated mock assertion to match new _stop_dvr_clients call signature. Summary: Adds an Extend action for in-progress recordings that bypasses Django signals and lets the running task dynamically pick up the new end_time via its 2-second DB polling loop. Fixes cross-recording interference caused by send_websocket_update deadlocking the gevent event loop. Fixes an artwork race condition where run_recording overwrote concurrent prefetch data. Adds Redis-based keepalive health checks for non-owner workers to prevent DVR read timeouts during source transitions. Replaces manual fetchRecordings() calls throughout the frontend with a debounced WS-driven refresh. 53 new tests. --- apps/channels/api_views.py | 148 ++++++-- apps/channels/signals.py | 19 +- apps/channels/tasks.py | 162 ++++++--- apps/channels/tests/test_recording_extend.py | 235 ++++++++++++ .../tests/test_recording_scheduling.py | 7 +- .../tests/test_recording_stop_cancel.py | 341 ++++++++++++++++++ .../channels/tests/test_ts_proxy_keepalive.py | 324 +++++++++++++++++ apps/proxy/ts_proxy/client_manager.py | 14 +- apps/proxy/ts_proxy/stream_generator.py | 25 +- core/utils.py | 46 +-- frontend/src/WebSocket.jsx | 71 ++-- frontend/src/api.js | 14 + .../src/components/cards/RecordingCard.jsx | 55 ++- .../forms/ProgramRecordingModal.jsx | 13 +- .../forms/RecordingDetailsModal.jsx | 11 +- .../components/forms/RecurringRuleModal.jsx | 9 +- frontend/src/pages/Guide.jsx | 12 +- frontend/src/store/channels.jsx | 12 +- .../src/utils/cards/RecordingCardUtils.js | 4 + 19 files changed, 1328 insertions(+), 194 deletions(-) create mode 100644 apps/channels/tests/test_recording_extend.py create mode 100644 apps/channels/tests/test_recording_stop_cancel.py create mode 100644 apps/channels/tests/test_ts_proxy_keepalive.py diff --git a/apps/channels/api_views.py b/apps/channels/api_views.py index bd94b552..b2bc0e14 100644 --- a/apps/channels/api_views.py +++ b/apps/channels/api_views.py @@ -11,7 +11,8 @@ from django.shortcuts import get_object_or_404, get_list_or_404 from django.db import transaction from django.db.models import Count, F from django.db.models import Q -import os, json, requests, logging, mimetypes +import os, json, requests, logging, mimetypes, threading +from datetime import timedelta from django.utils.http import http_date from urllib.parse import unquote from apps.accounts.permissions import ( @@ -2404,15 +2405,27 @@ class RecordingViewSet(viewsets.ModelViewSet): instance.custom_properties = cp instance.save(update_fields=["custom_properties"]) - # Everything else (DVR client teardown, task revocation, WebSocket notification) - # is moved to a daemon thread so the API returns immediately. Callers were - # seeing 5-15 s delays because _stop_dvr_clients / revoke_task / async_to_sync - # have occasional slow paths (Redis timeouts, Celery control broadcasts, etc.). + # Send the WebSocket notification before returning the response. + # send_websocket_update is gevent-safe (offloads async_to_sync to a + # real OS thread when monkey-patching is active). channel_uuid = str(instance.channel.uuid) recording_id = instance.id task_id = instance.task_id channel_name = instance.channel.name + try: + from core.utils import send_websocket_update + send_websocket_update('updates', 'update', { + "success": True, + "type": "recording_stopped", + "channel": channel_name, + }) + except Exception: + pass + + # DVR client teardown and task revocation are deferred to a daemon thread + # because they have occasional slow paths (Redis timeouts, Celery control + # broadcasts) that would otherwise add 5-15 s to the HTTP response time. def _background_stop(): try: stopped = _stop_dvr_clients(channel_uuid, recording_id=recording_id) @@ -2429,16 +2442,6 @@ class RecordingViewSet(viewsets.ModelViewSet): except Exception as e: logger.debug(f"Unable to revoke task for stopped recording: {e}") - try: - from core.utils import send_websocket_update - send_websocket_update('updates', 'update', { - "success": True, - "type": "recording_stopped", - "channel": channel_name, - }) - except Exception: - pass - # Close the DB connection opened by this thread so it isn't leaked try: from django.db import connection as _conn @@ -2446,52 +2449,106 @@ class RecordingViewSet(viewsets.ModelViewSet): except Exception: pass - import threading threading.Thread(target=_background_stop, daemon=True).start() return Response({"success": True, "status": "stopped"}) + @action(detail=True, methods=["post"], url_path="extend") + def extend(self, request, pk=None): + """Extend an in-progress recording's end_time without interrupting the stream. + + The running task re-reads end_time every ~2 s and adjusts its deadline + dynamically. The pre_save signal skips task revocation while the + recording status is 'recording'. + """ + instance = self.get_object() + cp = instance.custom_properties or {} + + if cp.get("status") in ("completed", "stopped", "interrupted"): + return Response( + {"success": False, "error": "Recording has already finished"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + try: + extra_minutes = int(request.data.get("extra_minutes", 0)) + except (TypeError, ValueError): + extra_minutes = 0 + + if extra_minutes <= 0: + return Response( + {"success": False, "error": "extra_minutes must be a positive integer"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + new_end_time = instance.end_time + timedelta(minutes=extra_minutes) + # Use queryset .update() to bypass pre_save/post_save signals. + # This avoids the pre_save signal revoking the scheduled/running + # Celery task. The running task's 2-second polling loop re-reads + # end_time from the DB and extends its deadline dynamically. + # If the task hasn't started yet (still in Beat's queue), it will + # read the updated end_time from the DB on its first poll cycle. + Recording.objects.filter(pk=instance.pk).update(end_time=new_end_time) + + try: + from core.utils import send_websocket_update + send_websocket_update('updates', 'update', { + "success": True, + "type": "recording_extended", + "recording_id": instance.id, + "new_end_time": new_end_time.isoformat(), + "extra_minutes": extra_minutes, + "channel": instance.channel.name, + }) + except Exception: + pass + + return Response({"success": True, "new_end_time": new_end_time.isoformat()}) + def destroy(self, request, *args, **kwargs): """Delete the Recording and ensure any active DVR client connection is closed. Also removes the associated file(s) from disk if present. + + Operation order matters for correctness: + 1. Delete the DB record first — run_recording's cancellation guard + (Recording.objects.filter(id=...).exists()) will now return False, + preventing it from saving 'interrupted' status or sending + recording_ended after the stream is torn down. + 2. Send recording_cancelled WebSocket immediately so the frontend + removes the card without waiting for the slow DVR client teardown. + 3. Spawn a background thread to stop the DVR client and delete files. + This mirrors the stop() endpoint's approach and avoids the 5-15 s + delay that _stop_dvr_clients() can introduce. """ instance = self.get_object() + recording_id = instance.pk channel_name = instance.channel.name - # Capture paths before deletion + # Capture state before the DB row is deleted cp = instance.custom_properties or {} rec_status = cp.get("status", "") - - # Only stop the DVR client if this recording is actively streaming. - # Stopping for completed/upcoming recordings would kill an unrelated - # in-progress recording on the same channel. - if rec_status == "recording": - try: - channel_uuid = str(instance.channel.uuid) - stopped = _stop_dvr_clients(channel_uuid) - if stopped: - logger.info(f"Stopped {stopped} DVR client(s) for channel {channel_uuid} due to recording cancellation") - except Exception as e: - logger.debug(f"Unable to stop DVR clients for cancelled recording: {e}") file_path = cp.get("file_path") temp_ts_path = cp.get("_temp_file_path") + channel_uuid = str(instance.channel.uuid) - # Perform DB delete first, then try to remove files + # 1. Delete the DB record (also fires post_delete → revoke_task_on_delete) response = super().destroy(request, *args, **kwargs) - # Notify frontends + # 2. Notify frontends immediately try: from core.utils import send_websocket_update send_websocket_update('updates', 'update', { "success": True, "type": "recording_cancelled", + "recording_id": recording_id, "channel": channel_name, "was_in_progress": rec_status == "recording", }) except Exception: pass + # 3. Defer slow teardown to a background thread library_dir = '/data' allowed_roots = ['/data/', library_dir.rstrip('/') + '/'] @@ -2505,8 +2562,33 @@ class RecordingViewSet(viewsets.ModelViewSet): except Exception as ex: logger.warning(f"Failed to delete recording artifact {path}: {ex}") - _safe_remove(file_path) - _safe_remove(temp_ts_path) + def _background_cancel(): + # Only stop the DVR client if the recording was actively streaming. + # Stopping for completed/upcoming recordings would kill an unrelated + # in-progress recording on the same channel. + if rec_status == "recording": + try: + stopped = _stop_dvr_clients(channel_uuid, recording_id=recording_id) + if stopped: + logger.info( + f"Stopped {stopped} DVR client(s) for channel {channel_uuid} due to recording cancellation" + ) + except Exception as e: + logger.debug(f"Unable to stop DVR clients for cancelled recording: {e}") + + # Best-effort file cleanup in case run_recording already exited + # before the DB delete. + _safe_remove(file_path) + _safe_remove(temp_ts_path) + + # Close the DB connection opened by this thread so it isn't leaked + try: + from django.db import connection as _conn + _conn.close() + except Exception: + pass + + threading.Thread(target=_background_cancel, daemon=True).start() return response diff --git a/apps/channels/signals.py b/apps/channels/signals.py index 00b8fb7c..a42b8c04 100644 --- a/apps/channels/signals.py +++ b/apps/channels/signals.py @@ -97,7 +97,7 @@ def schedule_recording_task(instance, eta=None): The task is stored in the database and dispatched by Celery Beat at the scheduled time with no countdown. This avoids the Redis visibility_timeout redelivery bug that caused duplicate recordings when using apply_async - with long countdowns. See https://github.com/Dispatcharr/Dispatcharr/issues/940 + with long countdowns. """ if eta is None: eta = instance.start_time @@ -136,8 +136,8 @@ def revoke_task(task_id): """Cancel a pending recording task. task_id is normally a PeriodicTask name (e.g. "dvr-recording-42"). - For backwards compatibility with task_ids stored before this fix - (Celery async-result UUIDs), falls back to AsyncResult.revoke(). + For backwards compatibility with legacy Celery async-result UUIDs, + falls back to AsyncResult.revoke(). """ if not task_id: return @@ -168,6 +168,12 @@ def revoke_old_task_on_update(sender, instance, **kwargs): old.end_time != instance.end_time or old.channel_id != instance.channel_id ): + # Do NOT revoke while the recording is actively streaming. + # run_recording re-reads end_time from the DB every ~2 s and extends + # its internal deadline dynamically — revoking here would kill the task. + old_status = (old.custom_properties or {}).get("status", "") + if old_status == "recording": + return revoke_task(old.task_id) instance.task_id = None except Recording.DoesNotExist: @@ -176,6 +182,13 @@ def revoke_old_task_on_update(sender, instance, **kwargs): @receiver(post_save, sender=Recording) def schedule_task_on_save(sender, instance, created, **kwargs): try: + # Skip processing for internal field-only saves (metadata updates, + # task_id assignment, end_time extensions) to prevent re-entrant + # artwork dispatch and redundant recording_updated WS events. + update_fields = kwargs.get('update_fields') + if not created and update_fields is not None and set(update_fields) <= {'custom_properties', 'task_id', 'end_time'}: + return + if not instance.task_id: start_time = instance.start_time diff --git a/apps/channels/tasks.py b/apps/channels/tasks.py index 87abb90d..26df8c78 100755 --- a/apps/channels/tasks.py +++ b/apps/channels/tasks.py @@ -1503,11 +1503,8 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): try: rec_check = Recording.objects.filter(id=recording_id).only("custom_properties").first() if not rec_check: - # Recording was deleted before this task got a chance to run (e.g. user - # cancelled from the Upcoming list, or destroy() ran concurrently). logger.info( - f"run_recording called for recording {recording_id} but it no longer exists — " - f"skipping (recording was cancelled before task started)." + f"run_recording called for recording {recording_id} but it no longer exists — skipping." ) return status = (rec_check.custom_properties or {}).get("status", "") @@ -1520,18 +1517,10 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): except Exception as e: logger.debug(f"Idempotency guard check failed (proceeding anyway): {e}") - # --- Clean up the one-off PeriodicTask that dispatched task --- + # --- Clean up the one-off PeriodicTask that dispatched this task --- try: - from django_celery_beat.models import ClockedSchedule as _CS, PeriodicTask as _PT - task_name = f"dvr-recording-{recording_id}" - try: - pt = _PT.objects.get(name=task_name) - old_clocked = pt.clocked - pt.delete() - if old_clocked and not _PT.objects.filter(clocked=old_clocked).exists(): - old_clocked.delete() - except _PT.DoesNotExist: - pass + from apps.channels.signals import revoke_task, _dvr_task_name + revoke_task(_dvr_task_name(recording_id)) except Exception as e: logger.debug(f"PeriodicTask cleanup failed (non-fatal): {e}") @@ -1541,8 +1530,7 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): end_time = datetime.fromisoformat(end_time_str) duration_seconds = int((end_time - start_time).total_seconds()) - # Build output paths from templates - # We need program info; will refine after we load Recording cp below + # Build output paths from templates (refined after loading Recording cp below) filename = None final_path = None temp_ts_path = None @@ -1575,6 +1563,16 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): recording_obj = None try: recording_obj = Recording.objects.get(id=recording_id) + # If the stop endpoint already wrote "stopped" before the task started, + # honor it instead of overwriting with "recording". + _pre_cp = recording_obj.custom_properties or {} + if _pre_cp.get("status") == "stopped": + logger.info( + f"run_recording {recording_id}: 'stopped' found in DB before stream started " + f"— task exits without connecting." + ) + return + # Prime custom_properties with file info/status cp = recording_obj.custom_properties or {} cp.update({ @@ -1817,8 +1815,38 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): except Exception: pass - recording_obj.custom_properties = cp + # Re-read from DB to preserve concurrent changes (e.g., artwork + # prefetch may have saved poster/rating info while resolving). + recording_obj.refresh_from_db() + fresh_cp = recording_obj.custom_properties or {} + + # If the stop endpoint set "stopped" while resolving, honor it. + if fresh_cp.get("status") == "stopped": + logger.info( + f"run_recording {recording_id}: 'stopped' found after metadata " + f"prep — task exits without streaming." + ) + return + + # Merge only the keys explicitly set into the fresh copy + for key in ("status", "started_at", "file_url", "output_file_url", + "file_name", "file_path", "_temp_file_path", + "poster_logo_id", "poster_url"): + if key in cp: + fresh_cp[key] = cp[key] + recording_obj.custom_properties = fresh_cp recording_obj.save(update_fields=["custom_properties"]) + + # Notify frontends so the tile picks up poster/metadata immediately + try: + from core.utils import send_websocket_update + send_websocket_update('updates', 'update', { + "success": True, + "type": "recording_updated", + "recording_id": recording_id, + }) + except Exception: + pass except Exception as e: logger.debug(f"Unable to prime Recording metadata: {e}") interrupted = False @@ -1888,7 +1916,12 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): elapsed = time.time() - started_at if elapsed > duration_seconds: break - # Continue draining the stream + # Continue draining the stream. + # Periodically poll the DB for a user-initiated stop so + # the task exits promptly if stop_client() was missed + # (e.g., fired before the DVR client was registered in Redis). + _stop_poll_interval = 2.0 # seconds between DB checks + _last_stop_poll = time.time() for chunk2 in response.iter_content(chunk_size=8192): if not chunk2: continue @@ -1897,6 +1930,50 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): elapsed = time.time() - started_at if elapsed > duration_seconds: break + # Periodic stopped-status + end_time extension check + _now = time.time() + if _now - _last_stop_poll >= _stop_poll_interval: + _last_stop_poll = _now + try: + _sc = Recording.objects.filter( + id=recording_id + ).only("custom_properties", "end_time").first() + if _sc is None: + # Recording was deleted — exit gracefully. + logger.info( + f"DVR recording {recording_id}: " + f"deleted — exiting stream loop" + ) + interrupted = False + break + if (_sc.custom_properties or {}).get("status") == "stopped": + logger.info( + f"DVR recording {recording_id}: stop requested " + f"— exiting stream loop early" + ) + break + # Check for extended end_time + try: + new_end = _sc.end_time + if new_end is not None: + from django.utils import timezone as _tz + if _tz.is_naive(new_end): + new_end = _tz.make_aware(new_end) + _ref = start_time + if _tz.is_naive(_ref): + _ref = _tz.make_aware(_ref) + new_duration = int((new_end - _ref).total_seconds()) + if new_duration > duration_seconds: + logger.info( + f"run_recording {recording_id}: " + f"end_time extended to {new_end}, " + f"new duration {new_duration}s" + ) + duration_seconds = new_duration + except Exception: + pass + except Exception: + pass break # exit outer for-loop once we switched to full drain # If we wrote any bytes, treat as success and stop trying candidates @@ -1933,16 +2010,14 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): interrupted = True interrupted_reason = f"stream_interrupted: {e}" break - # Also bail if the Recording was deleted (user cancelled before data arrived) - rec_exists = Recording.objects.filter(id=recording_id).exists() - if not rec_exists: - interrupted = True - interrupted_reason = "recording_deleted_during_connect" - break - # Check for "stopped" status even before data arrived + # Check if recording was deleted or stopped before data arrived try: rec_check = Recording.objects.filter(id=recording_id).only("custom_properties").first() - if rec_check and (rec_check.custom_properties or {}).get("status") == "stopped": + if rec_check is None: + interrupted = True + interrupted_reason = "recording_deleted_during_connect" + break + if (rec_check.custom_properties or {}).get("status") == "stopped": interrupted = False break except Exception: @@ -1951,15 +2026,6 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): if chosen_base is None and bytes_written == 0: interrupted = True interrupted_reason = f"no_stream_data: {last_error or 'all_bases_failed'}" - else: - # If we ended before reaching planned duration, record reason - actual_elapsed = 0 - try: - actual_elapsed = os.path.getsize(temp_ts_path) and (duration_seconds) # Best effort; we streamed until duration or disconnect above - except Exception: - pass - # We cannot compute accurate elapsed here; fine to leave as is - pass # If no bytes were written at all, check whether this was a deliberate stop or a # genuine failure. The exception handler above already sets interrupted=False when @@ -2024,10 +2090,13 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): logger.info(f"Recording {recording_id} was cancelled — skipping remux and metadata.") # Clean up any remaining temp or stub output files for _cleanup_path in [temp_ts_path, final_path]: + if not _cleanup_path: + continue try: - if _cleanup_path and os.path.exists(_cleanup_path): - os.remove(_cleanup_path) - logger.info(f"Cleaned up cancelled recording artifact: {_cleanup_path}") + os.remove(_cleanup_path) + logger.info(f"Cleaned up cancelled recording artifact: {_cleanup_path}") + except FileNotFoundError: + pass except Exception: pass return @@ -2138,12 +2207,14 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str): cp = recording_obj.custom_properties or {} cp["ended_at"] = str(datetime.now()) - # Determine final status. - # A deliberate user stop ("stopped" in DB) always wins over any interrupted flag — - # this handles races where the stream connection broke milliseconds before or - # after the stop endpoint committed its DB save. + # Final status priority: stopped > completed > interrupted. + # "stopped" is set by the stop endpoint before stream teardown, so + # refresh_from_db() above guarantees it is visible here. db_status_now = cp.get("status", "") - if db_status_now == "stopped" or not interrupted: + if db_status_now == "stopped": + # Deliberate user stop — preserve; do not overwrite with "completed". + cp.pop("interrupted_reason", None) + elif not interrupted: cp["status"] = "completed" cp.pop("interrupted_reason", None) else: @@ -2307,7 +2378,8 @@ def recover_recordings_on_startup(): for rec in upcoming: try: from django_celery_beat.models import PeriodicTask as _PT - task_name = f"dvr-recording-{rec.id}" + from apps.channels.signals import _dvr_task_name + task_name = _dvr_task_name(rec.id) if _PT.objects.filter(name=task_name).exists(): if rec.task_id != task_name: rec.task_id = task_name diff --git a/apps/channels/tests/test_recording_extend.py b/apps/channels/tests/test_recording_extend.py new file mode 100644 index 00000000..a083b6d3 --- /dev/null +++ b/apps/channels/tests/test_recording_extend.py @@ -0,0 +1,235 @@ +"""Tests for the Extend In-Progress Recording feature. + +Covers: + - extend() API endpoint (happy path and validation) + - pre_save signal guard: end_time change must NOT revoke a live recording + - pre_save signal guard: end_time change MUST still revoke an upcoming recording + - TOCTOU edge cases (extend on a completed/stopped/nonexistent recording) +""" +from datetime import timedelta +from unittest.mock import MagicMock, patch + +from django.test import TestCase +from django.utils import timezone +from rest_framework.test import APIRequestFactory, force_authenticate + +from apps.channels.models import Channel, Recording +from apps.channels.api_views import RecordingViewSet + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_admin(): + from django.contrib.auth import get_user_model + User = get_user_model() + u, _ = User.objects.get_or_create( + username="extend_test_admin", + defaults={"user_level": User.UserLevel.ADMIN}, + ) + u.set_password("pass") + u.save() + return u + + +# --------------------------------------------------------------------------- +# Extend endpoint tests +# --------------------------------------------------------------------------- + +class ExtendEndpointTests(TestCase): + """Tests for POST /api/channels/recordings/{id}/extend/""" + + def setUp(self): + self.channel = Channel.objects.create( + channel_number=88, name="Extend Test Channel" + ) + self.user = _make_admin() + self.factory = APIRequestFactory() + + def _extend(self, rec, extra_minutes): + request = self.factory.post( + f"/api/channels/recordings/{rec.id}/extend/", + {"extra_minutes": extra_minutes}, + format="json", + ) + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "extend"}) + return view(request, pk=rec.id) + + def _make_rec(self, status="recording"): + now = timezone.now() + return Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(hours=1), + end_time=now + timedelta(hours=1), + custom_properties={"status": status}, + ) + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_extend_updates_end_time_in_db(self, _ws): + rec = self._make_rec() + original_end = rec.end_time + response = self._extend(rec, 30) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.data.get("success")) + rec.refresh_from_db() + expected = original_end + timedelta(minutes=30) + delta = abs((rec.end_time - expected).total_seconds()) + self.assertLess(delta, 1, "end_time was not extended by the correct amount") + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_extend_stacks_multiple_extensions(self, _ws): + """Calling extend() twice adds both increments.""" + rec = self._make_rec() + original_end = rec.end_time + self._extend(rec, 15) + self._extend(rec, 30) + rec.refresh_from_db() + expected = original_end + timedelta(minutes=45) + delta = abs((rec.end_time - expected).total_seconds()) + self.assertLess(delta, 1) + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_extend_does_not_clear_task_id(self, _ws): + """The running Celery task must survive the DB save.""" + rec = self._make_rec() + rec.task_id = "dvr-recording-999" + rec.save(update_fields=["task_id"]) + self._extend(rec, 30) + rec.refresh_from_db() + self.assertEqual(rec.task_id, "dvr-recording-999") + + def test_extend_returns_400_if_finished(self): + """Cannot extend a completed, stopped, or interrupted recording.""" + for bad_status in ("completed", "stopped", "interrupted"): + with self.subTest(status=bad_status): + rec = self._make_rec(status=bad_status) + response = self._extend(rec, 30) + self.assertEqual(response.status_code, 400) + self.assertFalse(response.data.get("success")) + + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_extend_succeeds_before_task_sets_status(self, _ws): + """Extend must work when status is empty (task hasn't started yet).""" + rec = self._make_rec(status="") + response = self._extend(rec, 15) + self.assertEqual(response.status_code, 200) + rec.refresh_from_db() + expected = rec.end_time # already extended + self.assertTrue(response.data.get("success")) + + @patch("apps.channels.signals.revoke_task") + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_extend_bypasses_signals_no_revoke(self, _ws, mock_revoke): + """Extend uses .update() to bypass pre_save — revoke_task must never fire.""" + rec = self._make_rec(status="") + rec.task_id = "dvr-recording-500" + rec.save(update_fields=["task_id"]) + self._extend(rec, 15) + self._extend(rec, 30) + mock_revoke.assert_not_called() + rec.refresh_from_db() + self.assertEqual(rec.task_id, "dvr-recording-500") + + def test_extend_returns_400_for_zero_minutes(self): + response = self._extend(self._make_rec(), 0) + self.assertEqual(response.status_code, 400) + + def test_extend_returns_400_for_negative_minutes(self): + response = self._extend(self._make_rec(), -15) + self.assertEqual(response.status_code, 400) + + def test_extend_returns_400_for_non_numeric_minutes(self): + rec = self._make_rec() + request = self.factory.post( + f"/api/channels/recordings/{rec.id}/extend/", + {"extra_minutes": "lots"}, + format="json", + ) + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "extend"}) + response = view(request, pk=rec.id) + self.assertEqual(response.status_code, 400) + + def test_extend_returns_404_for_nonexistent_recording(self): + request = self.factory.post( + "/api/channels/recordings/999999/extend/", + {"extra_minutes": 30}, + format="json", + ) + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "extend"}) + response = view(request, pk=999999) + self.assertEqual(response.status_code, 404) + + +# --------------------------------------------------------------------------- +# pre_save signal guard tests +# --------------------------------------------------------------------------- + +class PreSaveExtendGuardTests(TestCase): + """The pre_save signal must NOT revoke a live recording when end_time changes, + but MUST still revoke a scheduled (upcoming) recording as before.""" + + def setUp(self): + self.channel = Channel.objects.create( + channel_number=77, name="Signal Guard Channel" + ) + + def _make_rec(self, status="", task_id="dvr-recording-42"): + now = timezone.now() + return Recording.objects.create( + channel=self.channel, + start_time=now + timedelta(hours=1), + end_time=now + timedelta(hours=2), + task_id=task_id, + custom_properties={"status": status} if status else {}, + ) + + @patch("apps.channels.signals.revoke_task") + def test_end_time_change_does_not_revoke_live_recording(self, mock_revoke): + """When status='recording', extending end_time must not call revoke_task.""" + rec = self._make_rec(status="recording", task_id="dvr-recording-42") + rec.end_time = rec.end_time + timedelta(minutes=30) + rec.save(update_fields=["end_time"]) + mock_revoke.assert_not_called() + + @patch("apps.channels.signals.revoke_task") + def test_task_id_preserved_after_extend_on_live_recording(self, mock_revoke): + """task_id must not be cleared for a live recording's end_time change.""" + rec = self._make_rec(status="recording", task_id="dvr-recording-42") + original_task_id = rec.task_id + rec.end_time = rec.end_time + timedelta(minutes=30) + rec.save(update_fields=["end_time"]) + rec.refresh_from_db() + self.assertEqual(rec.task_id, original_task_id) + + @patch("apps.channels.signals.revoke_task") + def test_end_time_change_still_revokes_upcoming_recording(self, mock_revoke): + """The guard must NOT apply to upcoming recordings — existing behavior preserved.""" + rec = self._make_rec(status="", task_id="dvr-recording-77") + rec.end_time = rec.end_time + timedelta(minutes=30) + rec.save(update_fields=["end_time"]) + mock_revoke.assert_called_once_with("dvr-recording-77") + + @patch("apps.channels.signals.revoke_task") + @patch("core.utils.send_websocket_update", side_effect=lambda *a, **kw: None) + def test_pre_save_guard_reads_db_status_not_memory_status(self, _ws, mock_revoke): + """pre_save reads status from DB (old object), not from the instance being saved.""" + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(hours=1), + end_time=now + timedelta(hours=1), + task_id="dvr-recording-66", + custom_properties={"status": "recording"}, + ) + # Simulate: DB status changes to 'completed' behind the instance's back + Recording.objects.filter(pk=rec.pk).update( + custom_properties={"status": "completed"} + ) + rec.end_time = rec.end_time + timedelta(minutes=30) + rec.save(update_fields=["end_time"]) + # revoke_task should be called because DB status is "completed", not "recording" + mock_revoke.assert_called_once_with("dvr-recording-66") diff --git a/apps/channels/tests/test_recording_scheduling.py b/apps/channels/tests/test_recording_scheduling.py index 56bdbe87..e9daecd7 100644 --- a/apps/channels/tests/test_recording_scheduling.py +++ b/apps/channels/tests/test_recording_scheduling.py @@ -1,7 +1,8 @@ """Tests for DVR recording scheduling with ClockedSchedule. - Prefers Celery ClockedScheduled over Redis time due to 3600s limitation on scheduled tasks; - scheduled tasks beyond 3600s cause Redis time to hand process between workers and duplicate jobs +Uses ClockedSchedule instead of apply_async with countdown because Redis +visibility_timeout (default 3600s) causes task redelivery for long countdowns, +leading to duplicate recordings. """ from datetime import timedelta from unittest.mock import patch, MagicMock @@ -501,7 +502,7 @@ class DestroyDvrClientIsolationTests(TestCase): custom_properties={"status": "recording"}, ) self._delete_recording(rec) - mock_stop.assert_called_once_with(str(self.channel.uuid)) + mock_stop.assert_called_once_with(str(self.channel.uuid), recording_id=rec.id) class PeriodicTaskCleanupOnExecutionTests(TestCase): diff --git a/apps/channels/tests/test_recording_stop_cancel.py b/apps/channels/tests/test_recording_stop_cancel.py new file mode 100644 index 00000000..962160ee --- /dev/null +++ b/apps/channels/tests/test_recording_stop_cancel.py @@ -0,0 +1,341 @@ +"""Tests for the DVR Stop/Cancel feature set. + +Covers: + - stop() endpoint + - destroy() was_in_progress field in recording_cancelled WebSocket event + - signals.py update_fields re-entrancy guard + - run_recording race guard before status write + - _stop_dvr_clients() DVR client isolation +""" +import time +from datetime import timedelta +from unittest.mock import MagicMock, AsyncMock, patch + +from django.test import TestCase +from django.utils import timezone +from rest_framework.test import APIRequestFactory, force_authenticate + +from apps.channels.models import Channel, Recording +from apps.channels.api_views import RecordingViewSet, _stop_dvr_clients + + +def _make_admin(): + from django.contrib.auth import get_user_model + User = get_user_model() + u, _ = User.objects.get_or_create( + username="stop_test_admin", + defaults={"user_level": User.UserLevel.ADMIN}, + ) + u.set_password("pass") + u.save() + return u + + +def _async_channel_layer_mock(): + layer = MagicMock() + layer.group_send = AsyncMock() + return layer + + +class StopEndpointTests(TestCase): + """Tests for POST /api/channels/recordings/{id}/stop/""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=99, name="Stop Test Channel") + self.user = _make_admin() + self.factory = APIRequestFactory() + + def _stop(self, rec): + request = self.factory.post(f"/api/channels/recordings/{rec.id}/stop/") + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "stop"}) + return view(request, pk=rec.id) + + def _make_rec(self, status="recording"): + now = timezone.now() + return Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(hours=1), + end_time=now + timedelta(hours=1), + custom_properties={"status": status}, + ) + + @patch("core.utils.send_websocket_update") + @patch("threading.Thread") + def test_stop_writes_status_to_db_before_returning(self, mock_thread, mock_ws): + """DB write is synchronous — run_recording polls for this.""" + mock_thread.return_value.start = MagicMock() + rec = self._make_rec() + response = self._stop(rec) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.data.get("success")) + rec.refresh_from_db() + self.assertEqual(rec.custom_properties.get("status"), "stopped") + + @patch("core.utils.send_websocket_update") + @patch("threading.Thread") + def test_stop_writes_stopped_at_timestamp(self, mock_thread, mock_ws): + mock_thread.return_value.start = MagicMock() + rec = self._make_rec() + self._stop(rec) + rec.refresh_from_db() + self.assertIn("stopped_at", rec.custom_properties) + + def test_stop_calls_stop_dvr_clients_in_background(self): + rec = self._make_rec() + with patch("apps.channels.api_views._stop_dvr_clients", return_value=1) as mock_stop, \ + patch("core.utils.send_websocket_update"): + self._stop(rec) + time.sleep(0.5) + self.assertTrue(mock_stop.called) + args, kwargs = mock_stop.call_args + actual_rec_id = kwargs.get("recording_id") or (args[1] if len(args) > 1 else None) + self.assertEqual(actual_rec_id, rec.id) + + def test_stop_returns_404_for_nonexistent(self): + request = self.factory.post("/api/channels/recordings/99999/stop/") + force_authenticate(request, user=self.user) + view = RecordingViewSet.as_view({"post": "stop"}) + self.assertEqual(view(request, pk=99999).status_code, 404) + + @patch("core.utils.send_websocket_update") + @patch("threading.Thread") + def test_stop_idempotent_on_already_stopped(self, mock_thread, mock_ws): + mock_thread.return_value.start = MagicMock() + rec = self._make_rec(status="stopped") + self.assertEqual(self._stop(rec).status_code, 200) + + +class CancelDestroyWasInProgressTests(TestCase): + """was_in_progress field in the recording_cancelled WebSocket event.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=98, name="Cancel Test Channel") + self.user = _make_admin() + self.factory = APIRequestFactory() + + def _delete(self, rec): + request = self.factory.delete(f"/api/channels/recordings/{rec.id}/") + force_authenticate(request, user=self.user) + return RecordingViewSet.as_view({"delete": "destroy"})(request, pk=rec.id) + + @patch("apps.channels.api_views._stop_dvr_clients", return_value=1) + @patch("core.utils.send_websocket_update") + def test_in_progress_sends_was_in_progress_true(self, mock_ws, _): + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(minutes=10), + end_time=now + timedelta(hours=1), + custom_properties={"status": "recording"}, + ) + self._delete(rec) + payload = mock_ws.call_args[0][2] + self.assertEqual(payload["type"], "recording_cancelled") + self.assertTrue(payload["was_in_progress"]) + + @patch("core.utils.send_websocket_update") + def test_completed_sends_was_in_progress_false(self, mock_ws): + rec = Recording.objects.create( + channel=self.channel, + start_time=timezone.now() - timedelta(hours=2), + end_time=timezone.now() - timedelta(hours=1), + custom_properties={"status": "completed"}, + ) + self._delete(rec) + self.assertFalse(mock_ws.call_args[0][2]["was_in_progress"]) + + +class SignalUpdateFieldsReentrancyGuardTests(TestCase): + """update_fields guard in schedule_task_on_save prevents redundant WS events.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=97, name="Signal Guard Channel") + + def _create_upcoming(self): + future = timezone.now() + timedelta(hours=2) + return Recording.objects.create( + channel=self.channel, start_time=future, + end_time=future + timedelta(hours=1), custom_properties={}, + ) + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_custom_properties_save_skips_artwork(self, mock_artwork): + rec = self._create_upcoming() + mock_artwork.reset_mock() + rec.custom_properties = {"poster_url": "https://example.com/p.jpg"} + rec.save(update_fields=["custom_properties"]) + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_task_id_save_skips_artwork(self, mock_artwork): + rec = self._create_upcoming() + mock_artwork.reset_mock() + rec.task_id = "dvr-recording-999" + rec.save(update_fields=["task_id"]) + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_combined_metadata_save_skips_artwork(self, mock_artwork): + rec = self._create_upcoming() + mock_artwork.reset_mock() + rec.task_id = "dvr-recording-1000" + rec.custom_properties = {"poster_url": "x"} + rec.save(update_fields=["custom_properties", "task_id"]) + mock_artwork.apply_async.assert_not_called() + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_creation_dispatches_artwork(self, mock_artwork): + mock_artwork.apply_async.return_value = MagicMock() + self._create_upcoming() + self.assertTrue(mock_artwork.apply_async.called) + + @patch("apps.channels.signals.prefetch_recording_artwork") + def test_scheduling_field_update_dispatches_artwork(self, mock_artwork): + """save(update_fields=['start_time']) is not a metadata save — dispatch runs.""" + mock_artwork.apply_async.return_value = MagicMock() + rec = self._create_upcoming() + mock_artwork.reset_mock() + future = timezone.now() + timedelta(hours=3) + rec.start_time = future + rec.end_time = future + timedelta(hours=1) + rec.save(update_fields=["start_time", "end_time"]) + mock_artwork.apply_async.assert_called() + + +class RunRecordingRaceGuardTests(TestCase): + """Race guard: stop() fires between idempotency check and status write.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=96, name="Race Guard Channel") + + def test_race_guard_exits_when_stopped_at_db_read(self): + """If Recording.objects.get() shows 'stopped', the task must exit + without writing 'recording' to the DB.""" + from apps.channels.tasks import run_recording as run_rec + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(minutes=1), + end_time=now + timedelta(hours=1), + custom_properties={}, + ) + mock_layer = _async_channel_layer_mock() + original_get = Recording.objects.get + + def patched_get(*args, **kwargs): + obj = original_get(*args, **kwargs) + if kwargs.get("id") == rec.id or (args and args[0] == rec.id): + obj.custom_properties = {"status": "stopped"} + return obj + + with patch("apps.channels.tasks.get_channel_layer", return_value=mock_layer), \ + patch("core.utils.log_system_event", side_effect=Exception("skip")), \ + patch.object(Recording.objects, "get", side_effect=patched_get): + result = run_rec( + rec.id, self.channel.id, str(rec.start_time), str(rec.end_time), + ) + + self.assertIsNone(result) + rec.refresh_from_db() + self.assertNotEqual( + rec.custom_properties.get("status"), "recording", + "Race guard failed: task overwrote 'stopped' with 'recording'", + ) + + def test_idempotency_guard_catches_stopped_before_channel_layer(self): + """When status='stopped' at the idempotency check, get_channel_layer is never called.""" + from apps.channels.tasks import run_recording as run_rec + now = timezone.now() + rec = Recording.objects.create( + channel=self.channel, + start_time=now - timedelta(minutes=5), + end_time=now + timedelta(hours=1), + custom_properties={"status": "stopped"}, + ) + with patch("apps.channels.tasks.get_channel_layer") as mock_get_layer: + result = run_rec( + rec.id, self.channel.id, str(rec.start_time), str(rec.end_time), + ) + self.assertIsNone(result) + mock_get_layer.assert_not_called() + + +class StopDvrClientsTests(TestCase): + """_stop_dvr_clients() DVR client isolation.""" + + def setUp(self): + self.channel = Channel.objects.create(channel_number=95, name="DVR Clients Channel") + self._redis = "core.utils.RedisClient" + self._sc = "apps.proxy.ts_proxy.services.channel_service.ChannelService.stop_client" + self._sch = "apps.proxy.ts_proxy.services.channel_service.ChannelService.stop_channel" + + def _mock_redis(self, client_ids, ua_map): + r = MagicMock() + r.smembers.return_value = {c.encode() for c in client_ids} + def hget_side(key, field): + ks = key if isinstance(key, str) else key.decode("utf-8", errors="replace") + for cid, ua in ua_map.items(): + if cid in ks: + return ua.encode() if isinstance(ua, str) else ua + return b"" + r.hget.side_effect = hget_side + return r + + def test_returns_zero_when_redis_none(self): + with patch(self._redis) as rc: + rc.get_client.return_value = None + self.assertEqual(_stop_dvr_clients(str(self.channel.uuid)), 0) + + def test_stops_only_matching_client_when_recording_id_given(self): + r = self._mock_redis( + ["client-a", "client-b"], + {"client-a": "Dispatcharr-DVR/recording-42", + "client-b": "Dispatcharr-DVR/recording-99"}, + ) + with patch(self._redis) as rc, patch(self._sc) as sc: + rc.get_client.return_value = r + result = _stop_dvr_clients(str(self.channel.uuid), recording_id=42) + self.assertEqual(result, 1) + stopped = [c[0][1] for c in sc.call_args_list] + self.assertIn("client-a", stopped) + self.assertNotIn("client-b", stopped) + + def test_stops_all_dvr_clients_without_recording_id(self): + r = self._mock_redis( + ["client-a", "client-b"], + {"client-a": "Dispatcharr-DVR/recording-42", + "client-b": "Dispatcharr-DVR/recording-99"}, + ) + with patch(self._redis) as rc, patch(self._sc) as sc: + rc.get_client.return_value = r + result = _stop_dvr_clients(str(self.channel.uuid)) + self.assertEqual(result, 2) + + def test_skips_non_dvr_clients(self): + r = self._mock_redis( + ["viewer", "dvr-client"], + {"viewer": "Mozilla/5.0", "dvr-client": "Dispatcharr-DVR/recording-1"}, + ) + with patch(self._redis) as rc, patch(self._sc) as sc: + rc.get_client.return_value = r + result = _stop_dvr_clients(str(self.channel.uuid)) + self.assertEqual(result, 1) + stopped = [c[0][1] for c in sc.call_args_list] + self.assertNotIn("viewer", stopped) + + def test_returns_zero_for_empty_channel(self): + r = MagicMock() + r.smembers.return_value = set() + with patch(self._redis) as rc, patch(self._sc) as sc: + rc.get_client.return_value = r + self.assertEqual(_stop_dvr_clients(str(self.channel.uuid)), 0) + sc.assert_not_called() + + def test_never_calls_stop_channel(self): + """Must not stop the whole channel proxy — only individual clients.""" + r = self._mock_redis(["dvr-1"], {"dvr-1": "Dispatcharr-DVR/recording-1"}) + with patch(self._redis) as rc, patch(self._sc), patch(self._sch) as sch: + rc.get_client.return_value = r + _stop_dvr_clients(str(self.channel.uuid)) + sch.assert_not_called() diff --git a/apps/channels/tests/test_ts_proxy_keepalive.py b/apps/channels/tests/test_ts_proxy_keepalive.py new file mode 100644 index 00000000..db388b3d --- /dev/null +++ b/apps/channels/tests/test_ts_proxy_keepalive.py @@ -0,0 +1,324 @@ +"""Tests for ts_proxy keepalive and stats-update behavior. + +Covers: + - stream_generator._should_send_keepalive() owner vs non-owner worker paths + - stream_generator._should_send_keepalive() Redis last_data health check + - client_manager._do_stats_update() error handling and WebSocket dispatch + - client_manager.remove_client() non-blocking stats update + - Keepalive/DVR-timeout timing invariants +""" +import threading +import time +from unittest.mock import MagicMock, patch + +from django.test import TestCase + + +# --------------------------------------------------------------------------- +# _should_send_keepalive: owner worker path +# --------------------------------------------------------------------------- + +class OwnerWorkerKeepaliveTests(TestCase): + """Owner worker has a stream_manager; keepalive logic uses it directly.""" + + def _make_generator(self, healthy, at_buffer_head, consecutive_empty): + from apps.proxy.ts_proxy.stream_generator import StreamGenerator + gen = StreamGenerator.__new__(StreamGenerator) + gen.channel_id = "00000000-0000-0000-0000-000000000001" + gen.client_id = "test-client" + + buffer = MagicMock() + buffer.index = 10 if at_buffer_head else 100 + gen.local_index = 10 + gen.buffer = buffer + + stream_manager = MagicMock() + stream_manager.healthy = healthy + gen.stream_manager = stream_manager + + gen.consecutive_empty = consecutive_empty + return gen + + def test_owner_healthy_returns_false(self): + """Owner worker, healthy stream -> no keepalive.""" + gen = self._make_generator(healthy=True, at_buffer_head=True, consecutive_empty=10) + self.assertFalse(gen._should_send_keepalive(gen.local_index)) + + def test_owner_unhealthy_at_head_returns_true(self): + """Owner worker, unhealthy stream, at buffer head -> send keepalive.""" + gen = self._make_generator(healthy=False, at_buffer_head=True, consecutive_empty=10) + self.assertTrue(gen._should_send_keepalive(gen.local_index)) + + def test_owner_unhealthy_not_at_head_returns_false(self): + """Owner worker, unhealthy stream, but NOT at buffer head -> no keepalive.""" + gen = self._make_generator(healthy=False, at_buffer_head=False, consecutive_empty=10) + self.assertFalse(gen._should_send_keepalive(gen.local_index)) + + def test_owner_insufficient_consecutive_empty_returns_false(self): + """Owner worker, unhealthy, at head but consecutive_empty < 5 -> no keepalive.""" + gen = self._make_generator(healthy=False, at_buffer_head=True, consecutive_empty=3) + self.assertFalse(gen._should_send_keepalive(gen.local_index)) + + def test_owner_exactly_5_consecutive_empty_returns_true(self): + """consecutive_empty == 5 is the minimum threshold.""" + gen = self._make_generator(healthy=False, at_buffer_head=True, consecutive_empty=5) + self.assertTrue(gen._should_send_keepalive(gen.local_index)) + + +# --------------------------------------------------------------------------- +# _should_send_keepalive: non-owner worker path +# --------------------------------------------------------------------------- + +class NonOwnerWorkerKeepaliveTests(TestCase): + """Non-owner worker has stream_manager=None; health determined from Redis.""" + + def _make_generator(self, consecutive_empty=10): + from apps.proxy.ts_proxy.stream_generator import StreamGenerator + gen = StreamGenerator.__new__(StreamGenerator) + gen.channel_id = "00000000-0000-0000-0000-000000000002" + gen.client_id = "test-client-nonowner" + + buffer = MagicMock() + buffer.index = 10 + gen.local_index = 10 + gen.buffer = buffer + + gen.stream_manager = None # non-owner worker + gen.consecutive_empty = consecutive_empty + return gen + + def _mock_proxy_server(self, last_data_value): + """Return a mock ProxyServer with a redis_client pre-configured.""" + server = MagicMock() + redis_client = MagicMock() + server.redis_client = redis_client + redis_client.get.return_value = last_data_value + return server + + def test_non_owner_fresh_data_returns_false(self): + """Non-owner, last_data < 10s ago -> stream healthy -> no keepalive.""" + gen = self._make_generator() + fresh_ts = str(time.time() - 2.0).encode() + server = self._mock_proxy_server(fresh_ts) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertFalse(result, "Fresh data should NOT trigger keepalive") + + def test_non_owner_stale_data_returns_true(self): + """Non-owner, last_data >= 10s ago -> stream unhealthy -> send keepalive.""" + gen = self._make_generator() + stale_ts = str(time.time() - 12.0).encode() + server = self._mock_proxy_server(stale_ts) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertTrue(result, "Stale data (12s) should trigger keepalive") + + def test_non_owner_exactly_at_timeout_returns_true(self): + """Data age exactly equal to CONNECTION_TIMEOUT (10s) -> send keepalive.""" + gen = self._make_generator() + ts = str(time.time() - 10.0).encode() + server = self._mock_proxy_server(ts) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertTrue(result, "Data at exactly timeout threshold should trigger keepalive") + + def test_non_owner_no_redis_key_returns_true(self): + """Non-owner, last_data key missing from Redis -> assume unhealthy.""" + gen = self._make_generator() + server = self._mock_proxy_server(None) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertTrue(result, "Missing last_data key should trigger keepalive") + + def test_non_owner_redis_client_none_returns_false(self): + """Non-owner, redis_client is None (disconnected) -> conservative, no keepalive.""" + gen = self._make_generator() + server = MagicMock() + server.redis_client = None + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertFalse(result, "No redis_client -> conservative, no keepalive") + + def test_non_owner_redis_exception_returns_false(self): + """Non-owner, Redis raises an exception -> conservative, no keepalive.""" + gen = self._make_generator() + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.side_effect = Exception("Redis error") + result = gen._should_send_keepalive(gen.local_index) + + self.assertFalse(result, "Redis error -> conservative, no keepalive") + + def test_non_owner_not_at_buffer_head_returns_false(self): + """Non-owner, NOT at buffer head -> no keepalive regardless of Redis.""" + gen = self._make_generator() + gen.buffer.index = 100 # far ahead of local_index=10 + server = self._mock_proxy_server(None) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertFalse(result) + + def test_non_owner_insufficient_consecutive_empty_returns_false(self): + """Non-owner, at head, but consecutive_empty < 5 -> no keepalive.""" + gen = self._make_generator(consecutive_empty=2) + stale_ts = str(time.time() - 30.0).encode() + server = self._mock_proxy_server(stale_ts) + + with patch("apps.proxy.ts_proxy.stream_generator.ProxyServer") as MockPS: + MockPS.get_instance.return_value = server + result = gen._should_send_keepalive(gen.local_index) + + self.assertFalse(result) + + +# --------------------------------------------------------------------------- +# _do_stats_update: error handling and WebSocket dispatch +# --------------------------------------------------------------------------- + +class DoStatsUpdateTests(TestCase): + """_do_stats_update runs the actual Redis scan + WebSocket call.""" + + def _make_client_manager(self): + from apps.proxy.ts_proxy.client_manager import ClientManager + cm = ClientManager.__new__(ClientManager) + cm.channel_id = "00000000-0000-0000-0000-000000000004" + cm._heartbeat_running = False + return cm + + def test_do_stats_update_calls_send_websocket_update(self): + """_do_stats_update must call send_websocket_update with channel_stats.""" + cm = self._make_client_manager() + + mock_redis = MagicMock() + mock_redis.scan.return_value = (0, []) + + with patch("apps.proxy.ts_proxy.client_manager.send_websocket_update") as mock_ws, \ + patch("redis.Redis.from_url", return_value=mock_redis): + cm._do_stats_update() + + mock_ws.assert_called_once() + event_type = mock_ws.call_args[0][1] + self.assertEqual(event_type, "update") + payload = mock_ws.call_args[0][2] + self.assertEqual(payload["type"], "channel_stats") + + def test_do_stats_update_does_not_raise_on_redis_error(self): + """Redis failure must be swallowed (logged), not propagated.""" + cm = self._make_client_manager() + + with patch("redis.Redis.from_url", side_effect=Exception("Redis down")): + try: + cm._do_stats_update() + except Exception as e: + self.fail(f"_do_stats_update raised an exception: {e}") + + def test_do_stats_update_scans_channel_client_keys(self): + """Must scan for ts_proxy:channel:*:clients pattern.""" + cm = self._make_client_manager() + + mock_redis = MagicMock() + mock_redis.scan.return_value = (0, []) + + with patch("apps.proxy.ts_proxy.client_manager.send_websocket_update"), \ + patch("redis.Redis.from_url", return_value=mock_redis): + cm._do_stats_update() + + scan_call = mock_redis.scan.call_args + self.assertIn("ts_proxy:channel:*:clients", str(scan_call)) + + +# --------------------------------------------------------------------------- +# Integration: remove_client must not block on WebSocket +# --------------------------------------------------------------------------- + +class ClientRemoveIntegrationTests(TestCase): + """When remove_client() fires, _trigger_stats_update must not block.""" + + def test_remove_client_does_not_block_on_websocket(self): + """remove_client() must return quickly even if WebSocket is slow.""" + from apps.proxy.ts_proxy.client_manager import ClientManager + + cm = ClientManager.__new__(ClientManager) + cm.channel_id = "00000000-0000-0000-0000-000000000005" + cm._heartbeat_running = False + cm.clients = {"test-client-1"} + cm.last_heartbeat_time = {"test-client-1": time.time()} + cm.last_active_time = time.time() + cm.client_set_key = f"ts_proxy:channel:{cm.channel_id}:clients" + cm.client_ttl = 60 + cm.worker_id = "worker-1" + cm.proxy_server = MagicMock() + cm.proxy_server.am_i_owner.return_value = False + cm.lock = threading.Lock() + + mock_redis = MagicMock() + mock_redis.hgetall.return_value = {b"ip_address": b"127.0.0.1"} + mock_redis.scard.return_value = 1 + cm.redis_client = mock_redis + + slow_ws_called = threading.Event() + + def slow_websocket(*args, **kwargs): + time.sleep(2.0) + slow_ws_called.set() + + start = time.time() + with patch("apps.proxy.ts_proxy.client_manager.send_websocket_update", side_effect=slow_websocket): + cm.remove_client("test-client-1") + elapsed = time.time() - start + + self.assertLess(elapsed, 1.0, + f"remove_client() blocked for {elapsed:.2f}s waiting for WebSocket " + f"(should dispatch to background thread and return immediately)") + + +# --------------------------------------------------------------------------- +# DVR timeout threshold vs keepalive timing +# --------------------------------------------------------------------------- + +class KeepaliveTimingTests(TestCase): + """Verify that keepalive threshold gives sufficient margin before DVR timeout.""" + + def test_keepalive_threshold_less_than_dvr_timeout(self): + """CONNECTION_TIMEOUT (keepalive trigger) must be < DVR read timeout (15s).""" + from apps.proxy.config import TSConfig as Config + connection_timeout = getattr(Config, "CONNECTION_TIMEOUT", 10) + dvr_read_timeout = 15 # hard-coded in run_recording: timeout=(10, 15) + self.assertLess( + connection_timeout, + dvr_read_timeout, + f"CONNECTION_TIMEOUT ({connection_timeout}s) must be < DVR timeout ({dvr_read_timeout}s) " + f"so keepalives fire before DVR times out", + ) + + def test_keepalive_interval_is_short(self): + """KEEPALIVE_INTERVAL must be short enough to send multiple keepalives in the gap.""" + from apps.proxy.config import TSConfig as Config + interval = getattr(Config, "KEEPALIVE_INTERVAL", 0.5) + connection_timeout = getattr(Config, "CONNECTION_TIMEOUT", 10) + remaining_window = 15 - connection_timeout + self.assertGreater( + remaining_window / interval, + 3, + f"KEEPALIVE_INTERVAL ({interval}s) is too long: only " + f"{remaining_window/interval:.1f} keepalives would fit in the " + f"{remaining_window}s window before DVR timeout", + ) diff --git a/apps/proxy/ts_proxy/client_manager.py b/apps/proxy/ts_proxy/client_manager.py index a361bfa1..3351cf56 100644 --- a/apps/proxy/ts_proxy/client_manager.py +++ b/apps/proxy/ts_proxy/client_manager.py @@ -43,14 +43,20 @@ class ClientManager: self._registered_clients = set() # Track already registered client IDs def _trigger_stats_update(self): - """Trigger a channel stats update via WebSocket""" + """Trigger a channel stats update via WebSocket in a background thread. + + Offloaded so the caller is not blocked. send_websocket_update is + gevent-safe (offloads async_to_sync to a native OS thread). + """ + threading.Thread(target=self._do_stats_update, daemon=True).start() + + def _do_stats_update(self): + """Perform the stats update in the background.""" try: - # Import here to avoid potential import issues from apps.proxy.ts_proxy.channel_status import ChannelStatus import redis from django.conf import settings - # Get all channels from Redis using settings redis_url = getattr(settings, 'REDIS_URL', 'redis://localhost:6379/0') redis_client = redis.Redis.from_url(redis_url, decode_responses=True) all_channels = [] @@ -59,7 +65,6 @@ class ClientManager: while True: cursor, keys = redis_client.scan(cursor, match="ts_proxy:channel:*:clients", count=100) for key in keys: - # Extract channel ID from key parts = key.split(':') if len(parts) >= 4: ch_id = parts[2] @@ -70,7 +75,6 @@ class ClientManager: if cursor == 0: break - # Send WebSocket update using existing infrastructure send_websocket_update( "updates", "update", diff --git a/apps/proxy/ts_proxy/stream_generator.py b/apps/proxy/ts_proxy/stream_generator.py index 50404f1d..8ba38fc0 100644 --- a/apps/proxy/ts_proxy/stream_generator.py +++ b/apps/proxy/ts_proxy/stream_generator.py @@ -381,10 +381,29 @@ class StreamGenerator: """Determine if a keepalive packet should be sent.""" # Check if we're caught up to buffer head at_buffer_head = local_index >= self.buffer.index + if not at_buffer_head or self.consecutive_empty < 5: + return False - # If we're at buffer head and no data is coming, send keepalive - stream_healthy = self.stream_manager.healthy if self.stream_manager else True - return at_buffer_head and not stream_healthy and self.consecutive_empty >= 5 + if self.stream_manager is not None: + # Owner worker: use the in-memory health flag directly. + return not self.stream_manager.healthy + else: + # Non-owner worker: stream_manager only exists in the owner process. + # Approximate health from the Redis last_data timestamp; if stale + # beyond CONNECTION_TIMEOUT, send keepalives to prevent DVR timeout. + try: + proxy_server = ProxyServer.get_instance() + if proxy_server.redis_client: + raw = proxy_server.redis_client.get(RedisKeys.last_data(self.channel_id)) + if raw: + age = time.time() - float(raw) + timeout_threshold = getattr(Config, 'CONNECTION_TIMEOUT', 10) + return age >= timeout_threshold + # No timestamp in Redis → key missing or expired → unhealthy + return True + except Exception: + pass + return False def _is_ghost_client(self, local_index): """Check if this appears to be a ghost client (stuck but buffer advancing).""" diff --git a/core/utils.py b/core/utils.py index f0e02bb4..ef5aa702 100644 --- a/core/utils.py +++ b/core/utils.py @@ -294,30 +294,34 @@ def send_websocket_update(group_name, event_type, data, collect_garbage=False): """ Standardized function to send WebSocket updates with proper memory management. - Args: - group_name: The WebSocket group to send to (e.g. 'updates') - event_type: The type of message (e.g. 'update') - data: The data to send - collect_garbage: Whether to force garbage collection after sending + In uWSGI + gevent deployments, async_to_sync creates an asyncio event loop + whose native EpollSelector blocks the entire OS thread, freezing all gevent + greenlets in the worker. To avoid this, the actual send is offloaded to a + real OS thread from gevent's native threadpool when monkey-patching is active. """ channel_layer = get_channel_layer() - try: - async_to_sync(channel_layer.group_send)( - group_name, - { - 'type': event_type, - 'data': data - } - ) - except Exception as e: - logger.warning(f"Failed to send WebSocket update: {e}") - finally: - # Explicitly release references to help garbage collection - channel_layer = None + message = {'type': event_type, 'data': data} - # Force garbage collection if requested - if collect_garbage: - gc.collect() + def _do_send(): + try: + async_to_sync(channel_layer.group_send)(group_name, message) + except Exception as e: + logger.warning(f"Failed to send WebSocket update: {e}") + + try: + import gevent.monkey + if gevent.monkey.is_module_patched('threading'): + from gevent import get_hub + get_hub().threadpool.spawn(_do_send) + return + except Exception: + pass + + # Not in a gevent-patched environment — call directly + _do_send() + + if collect_garbage: + gc.collect() def send_websocket_event(event, success, data): """Acquire a lock to prevent concurrent task execution.""" diff --git a/frontend/src/WebSocket.jsx b/frontend/src/WebSocket.jsx index f595b747..e687198c 100644 --- a/frontend/src/WebSocket.jsx +++ b/frontend/src/WebSocket.jsx @@ -19,6 +19,21 @@ import useAuthStore from './store/auth'; export const WebsocketContext = createContext([false, () => {}, null]); +// Debounce: coalesces rapid recording WS events into a single fetchRecordings() +// call (400 ms window) to prevent redundant re-renders in the TV Guide. +let _recordingFetchTimer = null; +function scheduleRecordingFetch() { + if (_recordingFetchTimer) clearTimeout(_recordingFetchTimer); + _recordingFetchTimer = setTimeout(async () => { + _recordingFetchTimer = null; + try { + await useChannelsStore.getState().fetchRecordings(); + } catch (e) { + console.warn('Failed to refresh recordings:', e); + } + }, 400); +} + export const WebsocketProvider = ({ children }) => { const [isReady, setIsReady] = useState(false); const [val, setVal] = useState(null); @@ -193,9 +208,7 @@ export const WebsocketProvider = ({ children }) => { loading: false, autoClose: 4000, }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch {} + scheduleRecordingFetch(); } else if (status === 'skipped') { notifications.update({ id, @@ -205,9 +218,7 @@ export const WebsocketProvider = ({ children }) => { loading: false, autoClose: 3000, }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch {} + scheduleRecordingFetch(); } else if (status === 'error') { notifications.update({ id, @@ -217,9 +228,7 @@ export const WebsocketProvider = ({ children }) => { loading: false, autoClose: 6000, }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch {} + scheduleRecordingFetch(); } break; } @@ -508,19 +517,11 @@ export const WebsocketProvider = ({ children }) => { break; case 'recording_updated': - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on update:', e); - } + scheduleRecordingFetch(); break; case 'recordings_refreshed': - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on refreshed:', e); - } + scheduleRecordingFetch(); break; case 'recording_started': @@ -528,11 +529,7 @@ export const WebsocketProvider = ({ children }) => { title: 'Recording started!', message: `Started recording channel ${parsedEvent.data.channel}`, }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on start:', e); - } + scheduleRecordingFetch(); break; case 'recording_ended': @@ -540,11 +537,7 @@ export const WebsocketProvider = ({ children }) => { title: 'Recording finished!', message: `Stopped recording channel ${parsedEvent.data.channel}`, }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on end:', e); - } + scheduleRecordingFetch(); break; case 'recording_stopped': @@ -553,11 +546,11 @@ export const WebsocketProvider = ({ children }) => { message: `Recording stopped early for ${parsedEvent.data.channel || 'channel'}. Partial content has been saved.`, color: 'yellow', }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on stop:', e); - } + scheduleRecordingFetch(); + break; + + case 'recording_extended': + scheduleRecordingFetch(); break; case 'recording_cancelled': @@ -568,10 +561,12 @@ export const WebsocketProvider = ({ children }) => { : 'Recording deleted.', color: 'red', }); - try { - await useChannelsStore.getState().fetchRecordings(); - } catch (e) { - console.warn('Failed to refresh recordings on cancel:', e); + // Surgical removal by ID avoids a full fetchRecordings() re-render. + // Fall back to a full refresh if the ID is missing (e.g. older server). + if (parsedEvent.data.recording_id != null) { + useChannelsStore.getState().removeRecording(parsedEvent.data.recording_id); + } else { + scheduleRecordingFetch(); } break; diff --git a/frontend/src/api.js b/frontend/src/api.js index 4fbf1018..43aca702 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -2673,6 +2673,20 @@ export default class API { } } + static async extendRecording(id, extraMinutes) { + try { + const resp = await request(`${host}/api/channels/recordings/${id}/extend/`, { + method: 'POST', + body: JSON.stringify({ extra_minutes: extraMinutes }), + headers: { 'Content-Type': 'application/json' }, + }); + return resp; + } catch (e) { + errorNotification(`Failed to extend recording ${id}`, e); + throw e; + } + } + static async runComskip(recordingId) { try { const resp = await request( diff --git a/frontend/src/components/cards/RecordingCard.jsx b/frontend/src/components/cards/RecordingCard.jsx index 2f428768..2386748e 100644 --- a/frontend/src/components/cards/RecordingCard.jsx +++ b/frontend/src/components/cards/RecordingCard.jsx @@ -13,20 +13,21 @@ import { Box, Button, Card, - Center, Flex, Group, Image, + Menu, Modal, Stack, Text, Tooltip, } from '@mantine/core'; -import { AlertTriangle, Square, SquareX } from 'lucide-react'; +import { AlertTriangle, Plus, Square, SquareX } from 'lucide-react'; import RecordingSynopsis from '../RecordingSynopsis'; import { deleteRecordingById, deleteSeriesAndRule, + extendRecordingById, getPosterUrl, getRecordingUrl, getSeasonLabel, @@ -43,7 +44,6 @@ const RecordingCard = ({ onOpenRecurring, channel: channelProp = null, }) => { - const channels = useChannelsStore((s) => s.channels); const env_mode = useSettingsStore((s) => s.environment.env_mode); const showVideo = useVideoStore((s) => s.showVideo); const fetchRecordings = useChannelsStore((s) => s.fetchRecordings); @@ -118,6 +118,27 @@ const RecordingCard = ({ } }; + const handleExtend = async (minutes, e) => { + e?.stopPropagation?.(); + try { + await extendRecordingById(recording.id, minutes); + notifications.show({ + title: 'Recording extended', + message: `Added ${minutes} minutes to this recording`, + color: 'teal', + autoClose: 2000, + }); + } catch (error) { + console.error('Failed to extend recording', error); + notifications.show({ + title: 'Extension failed', + message: 'Could not extend the recording', + color: 'red', + autoClose: 3000, + }); + } + }; + // Stop / Cancel / Delete state and handlers const [cancelOpen, setCancelOpen] = React.useState(false); const [stopConfirmOpen, setStopConfirmOpen] = React.useState(false); @@ -312,6 +333,30 @@ const RecordingCard = ({ + {isInProgress && ( + + + + + e.stopPropagation()} + onClick={(e) => e.stopPropagation()} + > + + + + e.stopPropagation()}> + Extend recording by + handleExtend(15, e)}>+15 minutes + handleExtend(30, e)}>+30 minutes + handleExtend(60, e)}>+1 hour + + + + + )} {isInProgress && ( } {!isUpcoming && - customProps?.status === 'completed' && + (customProps?.status === 'completed' || customProps?.status === 'stopped' || customProps?.status === 'interrupted') && (!customProps?.comskip || customProps?.comskip?.status !== 'completed') && ( + @@ -385,11 +484,20 @@ const RecordingDetailsModal = ({ )} - {description && ( + {editing ? ( +