diff --git a/docs/internal/BACKLOG_RELIABILITY_2026-09-12.md b/docs/internal/BACKLOG_RELIABILITY_2026-09-12.md new file mode 100644 index 00000000..d03af502 --- /dev/null +++ b/docs/internal/BACKLOG_RELIABILITY_2026-09-12.md @@ -0,0 +1,106 @@ +# Reliability backlog work — 2026-09-12 + +Based on main `7f91155c9f7a0958fe6737b11c56d8090527779f` (0.41.17). +The affected cameras are unavailable in this workspace. No additional SQLite +error log or ODM capture is available beyond the issue reports. + +| Issue | Implemented or verified locally | Remaining evidence | +| --- | --- | --- | +| [#600](https://github.com/opensensor/lightNVR/issues/600) | Removed HLS `alarm()` calls and temporary SIGALRM/SIGSEGV dispositions; removed the freed-memory probe. Concurrent writer closure and full stream shutdown/restart pass. Fixed a codec metadata leak exposed by the lifecycle test. | The reported connection to a particular deadlock remains unproven. The system API suite completes. These tests do not establish freedom from every existing HLS context lifetime race. | +| [#603](https://github.com/opensensor/lightNVR/issues/603) | Parse the subscription destination from the XML response's `SubscriptionReference`, including arbitrary prefixes and escaped URL characters. Escape `wsa:To`. Reject empty HTTP 200 responses, disable curl signal use, and log action/curl code/HTTP code/elapsed time. Test dropped connections and subsequent subscription reuse. | Tapo C530WS model/hardware revision/firmware, working ODM exchanges, and matching lightNVR exchanges are still needed. Keep open; this does not establish that the report is a duplicate of #567 or that these changes fix its empty connection response. | +| [#580](https://github.com/opensensor/lightNVR/issues/580) | Removed whole-file cache eviction during `integrity_check`; retain full verification, copy-batch cache release, final eviction, shutdown abort and deadline checks. Schedule the next backup interval from completion and defer failed attempts without counting them as successful backups. Performance work remains linked to [#604](https://github.com/opensensor/lightNVR/issues/604). | Persistent timeline `SQLITE_IOERR` remains a separate investigation. Need the full failing operation and `code`/`extended_code`, with filesystem/device context. No database recovery or error suppression was added. | +| [#568](https://github.com/opensensor/lightNVR/issues/568) | Native video play/pause updates shared investigation playback. Apply shared changes before delayed native events can undo a pause. Automatic pauses during media reload, clip end or errors do not stop the shared clock. Browser test exercises real media events and reload. | Reporter already confirmed the cropping fix; no further cropping change was needed. | +| [#494](https://github.com/opensensor/lightNVR/issues/494) | Real RTSP input plus simulated ONVIF events produces a decodable detection-only MP4 containing footage before the trigger. Eight-second pre-buffer and three-second post-buffer are exercised. | Still needs footage from the affected camera on the current release. The synthetic source has a one-second GOP; it does not establish behavior for every camera's GOP, timestamps or reconnect history. | +| [#579](https://github.com/opensensor/lightNVR/issues/579) | Existing browser tests pass: 1×1 selects main, multi-cell selects sub, and fullscreen upgrades only its selected cell. No new stream-selection change needed. | Retest the affected camera with main-only recording/viewing, then add sub-stream consumers. Its later failure in the manufacturer app remains a separate symptom; browser tests cannot validate camera stability. | + +## HLS lifecycle validation + +`test_hls_cleanup` runs three writers through three closure/recreation cycles, +then three full unified HLS streams through two simultaneous restart cycles and +simultaneous shutdown. The writer test reopens finalized playlists and reads +media. The stream test paces local H.264 input to keep threads active during +shutdown. Link wrappers reject alarm use or signal-disposition changes. + +The expanded test initially found 981 leaked bytes across nine stream lifetimes. +LeakSanitizer traced them to codec extradata allocated during +`avformat_find_stream_info`. `comprehensive_ffmpeg_cleanup` discarded those +pointers and zeroed the codec parameters before FFmpeg could free them. It now +leaves demuxer-owned parameters intact for normal input closure. The test passes +with address/leak sanitizers enabled; the test does not suppress leak detection. + +## Backup measurement + +Reproduce on a disk-backed directory; `/tmp` on this workspace is tmpfs: + +```sh +python3 tests/database/benchmark_backup_verification.py build +``` + +The script creates its own indexed audit fixture from migration 0055, starts +each verification with cold file cache, measures `/proc/self/io` read bytes, and +removes the fixture. It never opens an application database. + +One local run with 100,000 rows and an 81,793,024-byte fixture: + +| Verification policy | Read bytes | Elapsed | Integrity result | +| --- | ---: | ---: | --- | +| Evict whole file every 100,000 VM operations | 178,323,456 | 1.235 s | ok | +| Retain cache during verification | 81,776,640 | 0.924 s | ok | + +An earlier 67 MB audit fixture showed 368 MB versus 67 MB read, and 3.437 s +versus 0.844 s. The amplification depends on table/index layout and available +memory; neither small local run predicts the reported 38–48-minute production +backup duration. The repeatable result is reduced rereading while retaining the +same integrity check. Verification may occupy more reclaimable OS page cache +until completion; the SQLite heap limit and copy-phase controls remain. + +## Actual ONVIF pre-buffer footage + +```sh +python3 tests/integration/synthetic-onvif-prebuffer.py --go2rtc /path/to/go2rtc +``` + +Requires the built application, frontend assets, FFmpeg and go2rtc. The harness +uses dedicated temporary files, loopback camera ports and separately owned +processes. It retains application/source logs, the MP4 and `result.json` in the +printed `/tmp/lightnvr-prebuffer-*` directory. + +The source changes from red to blue at media second 12. Motion begins after +second 15 and stops after second 18. Continuous recording is disabled, and the +harness checks that there was no recording before the event. The MP4 must start +red, end blue, and carry a pre-trigger interval in its metadata. This validates +the footage, independently of a settings value or packet-buffer count. + +The first successful local run produced a 32.2-second clip with a red first +frame (`255,0,0`) and a start time eight seconds before the simulated trigger. +Logs showed 92 buffered packets flushed, 91 written from a keyframe, and entry +into a three-second post-buffer. The total clip also includes the existing +15-second ONVIF motion hold and detection grace interval. + +The checked-in harness also passed after the final C changes: a 31.4-second +clip started red (`255,0,0`), ended blue (`0,0,255`), and began about 7.8 seconds +before the trigger. Its local artifacts are in +`/tmp/lightnvr-prebuffer-m2pnwzlh/result.json` and the sibling recording directory. + +## Checks + +Configured Debug build with tests enabled and SOD, LiteRT, MQTT and SSL disabled. +Frontend production build passed. Targeted C tests passed: + +```sh +cmake --build build -j8 --target lightnvr test_hls_cleanup \ + test_detection_system_onvif test_db_backup test_packet_buffer \ + test_api_handlers_system test_mp4_segment_timestamps +ctest --test-dir build --output-on-failure --timeout 30 \ + -R '^(test_hls_cleanup|test_detection_system_onvif|test_db_backup|test_packet_buffer|test_api_handlers_system|test_mp4_segment_timestamps)$' +``` + +Six C suites passed in 31.48 s; `test_api_handlers_system` completed in 20.67 s. +The ONVIF suite includes 14 cases. Investigation browser tests passed (2 cases), +as did the existing sub-stream/fullscreen tests (2 cases). Browser tests used +`LIGHTNVR_TEST_DIR`, `LIGHTNVR_TEST_CONFIG`, and `LIGHTNVR_TEST_BIN` pointing at +an isolated test instance, with `LIGHTNVR_SKIP_GO2RTC=1` for mocked live endpoints. + +These results support the local fixes and regression coverage. They do not +close the hardware-dependent reports or identify the persistent SQLite I/O +error's extended cause. diff --git a/src/core/daemon.c b/src/core/daemon.c index 41413aaf..d98346e0 100644 --- a/src/core/daemon.c +++ b/src/core/daemon.c @@ -178,20 +178,8 @@ static void daemon_signal_handler(int sig) { web_server_socket = -1; // Update the global reference } - // Deliberately NOT using alarm() here as a "force exit if shutdown - // hangs" watchdog: alarm() is a single process-wide timer, and this - // codebase's HLS writer/context-close code (hls_unified_thread.c, - // hls_writer.c) uses alarm() extensively as a short per-operation - // timeout (save disposition, alarm(N), do the call, alarm(0), - // restore disposition). Any one of those firing during shutdown - // would silently discard whatever time was left on an alarm set - // here, since alarm() has no pause/resume -- confirmed live via - // gdb: a shutdown that should have had ~570s left was killed at 45s - // by one of those unrelated short-lived alarms, in this exact - // handler, right after this comment block previously called - // alarm(570) here. main.c's start_shutdown_watchdog_thread() - // (a dedicated thread polling `running`, spawned once at startup) - // now owns this responsibility instead, immune to that collision. + // main.c's dedicated shutdown watchdog owns the deadline. Do not + // introduce a process-global alarm shared with unrelated threads. break; case SIGHUP: @@ -201,17 +189,8 @@ static void daemon_signal_handler(int sig) { break; case SIGALRM: - // No longer the shutdown watchdog (see main.c's - // start_shutdown_watchdog_thread()). Kept registered, and - // deliberately harmless, purely so SIGALRM has a caught - // (non-terminating) disposition as a safety net: the many - // hls_unified_thread.c/hls_writer.c call sites that use alarm() for - // their own short per-operation timeouts save whatever handler is - // installed here before temporarily switching it to SIG_IGN, and - // restore it afterward. If this weren't registered, SIGALRM's - // default disposition (process termination) would apply during any - // brief window where none of those local overrides happen to be - // active. + // Compatibility for externally delivered SIGALRM; HLS cleanup no + // longer arms alarms or changes the process signal dispositions. daemon_signal_safe_write("[DAEMON] Stray SIGALRM caught at top level (harmless, ignored)\n"); break; diff --git a/src/core/main.c b/src/core/main.c index 946fe431..b7085b7c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -182,19 +182,9 @@ static void signal_handler(int sig) { // request_background_abort() is async-signal-safe (atomic store only). request_background_abort(); - // Deliberately NOT using alarm() here as a "force exit if shutdown - // hangs" watchdog anymore -- see start_shutdown_watchdog_thread()'s - // comment for why: alarm() is a single process-wide timer, and this - // codebase's HLS writer/context-close code (hls_unified_thread.c, - // hls_writer.c) uses alarm() extensively as a short per-operation - // timeout (save disposition, alarm(N), do the call, alarm(0), restore - // disposition). Any one of those firing during shutdown would silently - // discard whatever time was left on an alarm set here, since alarm() - // has no pause/resume -- confirmed live via gdb: a shutdown that should - // have had ~570s left was killed at 45s by one of those unrelated - // short-lived alarms. A dedicated watchdog thread can't be clobbered - // this way. This handler now only sets flags and closes the listening - // socket, both async-signal-safe. + // The dedicated shutdown watchdog owns the deadline. Process-global + // alarm timers cannot safely implement independent thread timeouts. + // This handler only sets flags and closes the listening socket. if (web_server_socket >= 0) { close(web_server_socket); web_server_socket = -1; @@ -203,15 +193,9 @@ static void signal_handler(int sig) { // Alarm signal handler -- MUST ONLY use async-signal-safe functions. // -// No longer the shutdown watchdog (see start_shutdown_watchdog_thread()). -// Kept registered, and deliberately harmless, purely so SIGALRM has a -// caught (non-terminating) disposition as a safety net: the many -// hls_unified_thread.c/hls_writer.c call sites that use alarm() for their -// own short per-operation timeouts save whatever handler is installed here -// before temporarily switching it to SIG_IGN, and restore it afterward. If -// this weren't registered, SIGALRM's default disposition (process -// termination) would apply during any brief window where none of those -// local overrides happen to be active. +// Retain harmless handling of externally delivered SIGALRM for compatibility. +// HLS cleanup does not arm alarms or change process signal dispositions. +// Shutdown deadlines belong to start_shutdown_watchdog_thread(). static void alarm_handler(int sig) { (void)sig; signal_safe_write("[SIGNAL] Stray SIGALRM caught at top level (harmless, ignored)\n"); @@ -236,17 +220,8 @@ static void alarm_handler(int sig) { // by the time that elapses (this thread simply vanishes along with every // other thread on a normal exit, so it never fires in the common case). // -// Deliberately NOT alarm()-based: alarm() is a single process-wide timer, -// and hls_unified_thread.c/hls_writer.c use it extensively as a short -// per-operation timeout (save disposition, alarm(N), do the call, alarm(0), -// restore disposition) around individual writer/context close calls. Any -// one of those firing during shutdown discards whatever time was left on an -// outer alarm, since alarm() has no pause/resume -- confirmed live via gdb: -// a shutdown that should have had ~570s left was killed at 45s by one of -// those unrelated short-lived alarms. This thread runs independently of -// SIGALRM entirely, so none of that matters here. Unlike a signal handler, -// this runs in normal thread context, so log_error() and kill() are both -// safe to call directly. +// This dedicated thread keeps the deadline independent of process signals. +// It runs in normal thread context, so log_error() and kill() are safe here. static void *shutdown_watchdog_thread_func(void *arg) { (void)arg; bool shutdown_started = false; @@ -324,7 +299,7 @@ static void init_signals() { sigaction(SIGTERM, &sa, NULL); sigaction(SIGHUP, &sa, NULL); - // Set up alarm handler for phased forced exit + // Preserve harmless handling of externally delivered SIGALRM. struct sigaction sa_alarm; memset(&sa_alarm, 0, sizeof(sa_alarm)); sa_alarm.sa_handler = alarm_handler; @@ -1422,10 +1397,6 @@ int main(int argc, char *argv[]) { log_info("Stopping health check system..."); cleanup_health_check_system(); - // Cancel any pending alarm from signal_handler to prevent interference with cleanup - // alarm(0) cancels any previously set alarm - this is async-signal-safe - alarm(0); - // Block most signals during cleanup to prevent interruptions // But keep SIGUSR1, SIGALRM, and SIGKILL unblocked for emergency shutdown sigset_t block_mask, old_mask; @@ -1466,18 +1437,8 @@ int main(int argc, char *argv[]) { // Save the parent PID before it gets killed pid_t parent_pid = getppid(); - // Phase 1 must outlast the deliberate, non-abortable final backup - // shutdown_database() takes before exiting (observed up to ~9 - // minutes on this box's 2.6GB+ database) -- same reasoning as - // SHUTDOWN_WATCHDOG_TIMEOUT_SECONDS and the alarm() values in - // daemon.c/signal_handler(). This watchdog is fork+sleep()-based - // rather than alarm()-based, so unlike those it was never actually - // affected by the alarm()/SIGALRM collision with - // hls_unified_thread.c/hls_writer.c's per-operation timeouts -- - // it was simply always too short (30s) on its own, and was in fact - // the one actually killing every real shutdown after those other - // fixes landed, confirmed via its distinct "phase 1/2 timed out" - // log lines. + // Allow the same final-backup deadline as the thread watchdog. + // Large databases can take several minutes to copy and verify. sleep(SHUTDOWN_WATCHDOG_TIMEOUT_SECONDS); log_error("Cleanup process phase 1 timed out after %d seconds", SHUTDOWN_WATCHDOG_TIMEOUT_SECONDS); kill(parent_pid, SIGUSR1); // Send USR1 to parent to trigger emergency cleanup diff --git a/src/database/db_backup.c b/src/database/db_backup.c index bc080ea9..3952ef91 100644 --- a/src/database/db_backup.c +++ b/src/database/db_backup.c @@ -109,15 +109,13 @@ static void release_path_cache(const char *path) { } typedef struct { - int fd; - const char *path; bool abortable; const backup_deadline_t *deadline; bool deadline_hit; -} cache_release_progress_t; +} verification_progress_t; static int progress_during_verification(void *opaque) { - cache_release_progress_t *progress = (cache_release_progress_t *)opaque; + verification_progress_t *progress = (verification_progress_t *)opaque; /* Post-copy integrity_check scans the whole backup file page by page -- * on a multi-gigabyte database this alone can take as long as the copy * it's verifying, with no other abort point once sqlite3_backup_finish() @@ -134,9 +132,6 @@ static int progress_during_verification(void *opaque) { return 1; } } - if (progress && progress->fd >= 0) { - release_file_cache(progress->fd, progress->path); - } return 0; } @@ -470,18 +465,16 @@ int backup_database(const char *source_path, const char *dest_path, bool abortab goto cleanup; } - /* Retain the full integrity guarantee, but release clean pages as SQLite - * scans the backup. Without the progress callback this second full-file - * pass recreates the multi-gigabyte cache footprint bounded above. - * Registered whenever abortable even without a cache fd, since the - * abort-check matters independently of the cache-release optimization. */ - cache_release_progress_t verification_progress = { - .fd = dest_cache_fd, - .path = temp_path, + /* integrity_check revisits table/index pages; evicting the whole file + * every 100k VM instructions forces those pages to be read repeatedly. + * Keep the OS's reclaimable cache during verification and discard it at + * completion below. Copying still flushes/releases each bounded batch. + * The progress callback remains responsible for aborts and deadlines. */ + verification_progress_t verification_progress = { .abortable = abortable, .deadline = &deadline, }; - if (dest_cache_fd >= 0 || abortable) { + if (abortable) { sqlite3_progress_handler(dest_db, BACKUP_VERIFY_PROGRESS_OPS, progress_during_verification, &verification_progress); diff --git a/src/database/db_core.c b/src/database/db_core.c index 4d647aa2..76fc8172 100644 --- a/src/database/db_core.c +++ b/src/database/db_core.c @@ -44,6 +44,7 @@ static char db_backup_path[PATH_MAX] = {0}; // Last backup time static time_t last_backup_time = 0; +static time_t last_scheduled_backup_finished = 0; // Flag to indicate if WAL mode is enabled static bool wal_mode_enabled = false; @@ -512,15 +513,21 @@ int maybe_run_scheduled_database_backup(void) { time_t now = time(NULL); time_t interval_seconds = (time_t)g_config.db_backup_interval_minutes * 60; - if (last_backup_time != 0 && now - last_backup_time < interval_seconds) { + time_t last_cycle = last_scheduled_backup_finished > last_backup_time + ? last_scheduled_backup_finished : last_backup_time; + if (last_cycle != 0 && now - last_cycle < interval_seconds) { return 0; } - if (perform_database_backup_cycle("scheduled", true, true) != 0) { + int result = perform_database_backup_cycle("scheduled", true, true); + // Allow storage to recover after a slow or failed cycle. Failed attempts + // must not run again on every main-loop tick or count as usable backups. + last_scheduled_backup_finished = time(NULL); + if (result != 0) { return -1; } - last_backup_time = now; + last_backup_time = last_scheduled_backup_finished; return 0; } @@ -581,6 +588,7 @@ int init_database_ex(const char *db_path, unsigned flags) { log_info("Backup path set to: %s", db_backup_path); struct stat backup_stat; + last_scheduled_backup_finished = 0; if (stat(db_backup_path, &backup_stat) == 0) { last_backup_time = backup_stat.st_mtime; } else { diff --git a/src/video/ffmpeg_utils.c b/src/video/ffmpeg_utils.c index 6af242cc..c5e5b4b7 100644 --- a/src/video/ffmpeg_utils.c +++ b/src/video/ffmpeg_utils.c @@ -184,75 +184,13 @@ void comprehensive_ffmpeg_cleanup(AVFormatContext **input_ctx, AVCodecContext ** log_debug("Cleaned up AVCodecContext"); } - // Clean up input context with special handling for parsers and internal buffers + // The demuxer owns stream codec parameters, including extradata and + // channel layouts. Clearing those pointers before closing loses the + // allocations; let FFmpeg release them along with the input context. if (input_ctx && *input_ctx) { - AVFormatContext *ctx = *input_ctx; - - // MEMORY LEAK FIX: Manually clean up parsers and internal buffers - // This addresses the memory leaks in avformat_find_stream_info - if (ctx->nb_streams > 0) { - for (unsigned int i = 0; i < ctx->nb_streams; i++) { - if (ctx->streams[i]) { - // Clean up any codec parameters - if (ctx->streams[i]->codecpar) { - // We don't free codecpar directly as it's managed by the stream - // But we can clear any internal buffers - if (ctx->streams[i]->codecpar->extradata) { - // The extradata is freed by avformat_close_input, but we'll - // clear the pointer to prevent potential use-after-free - ctx->streams[i]->codecpar->extradata = NULL; - ctx->streams[i]->codecpar->extradata_size = 0; - } - - // CRITICAL FIX: Explicitly clean up any internal allocations made by avcodec_parameters_from_context - // This addresses the memory leak shown in Valgrind - AVCodecParameters *codecpar = ctx->streams[i]->codecpar; - if (codecpar) { - // Reset fields that might contain allocated memory - if (codecpar->extradata) { - codecpar->extradata = NULL; - codecpar->extradata_size = 0; - } - - // Clear any other fields that might have allocated memory - codecpar->ch_layout.u.mask = 0; - if (codecpar->ch_layout.nb_channels > 0) { - codecpar->ch_layout.nb_channels = 0; - } - - // ENHANCED FIX: Create a temporary codec context to force cleanup of internal allocations - // This is a workaround for FFmpeg's internal memory management issues - const AVCodec *codec = avcodec_find_decoder(codecpar->codec_id); - if (codec) { - AVCodecContext *temp_ctx = avcodec_alloc_context3(codec); - if (temp_ctx) { - // Copy parameters to the context - int ret = avcodec_parameters_to_context(temp_ctx, codecpar); - if (ret >= 0) { - // Now copy back to parameters - this will reallocate and clean up any leaks - avcodec_parameters_from_context(codecpar, temp_ctx); - } - // Free the temporary context - avcodec_free_context(&temp_ctx); - } - } - - // Zero out the entire structure to ensure no pointers remain - memset(codecpar, 0, sizeof(AVCodecParameters)); - } - } - } - } - } - - // Now use our safe cleanup function safe_avformat_cleanup(input_ctx); } - // Note: We're not using aggressive memory cleanup techniques here - // to avoid potential segmentation faults - // Instead, we rely on FFmpeg's own memory management - log_info("Comprehensive FFmpeg resource cleanup completed"); } diff --git a/src/video/hls/hls_unified_thread.c b/src/video/hls/hls_unified_thread.c index 10f196ec..cf47263c 100644 --- a/src/video/hls/hls_unified_thread.c +++ b/src/video/hls/hls_unified_thread.c @@ -21,8 +21,6 @@ #include #include #include -#include -#include #include #include #include @@ -73,16 +71,6 @@ static void global_ffmpeg_cleanup(void) { log_info("Global FFmpeg cleanup called during program exit"); - // CRITICAL FIX: Use try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - // Call FFmpeg's internal memory cleanup functions - safely log_debug("Calling avformat_network_deinit() during global cleanup"); avformat_network_deinit(); @@ -102,10 +90,6 @@ static void global_ffmpeg_cleanup(void) { av_free(dummy); } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - log_info("Global FFmpeg cleanup completed"); in_global_cleanup = 0; } @@ -242,50 +226,7 @@ static bool is_context_already_freed(void *ctx) { pthread_mutex_unlock(&freed_contexts_mutex); - // If not found in the freed contexts list, perform additional checks - if (!result) { - // Check if the memory appears to be invalid - // This is a heuristic to detect freed memory - bool appears_invalid = false; - - // Try to access the first few bytes to see if they're zeroed out - // This might indicate that the memory has been freed - const unsigned char *ptr = (const unsigned char *)ctx; - bool all_zeros = true; - - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - - // Check the first few bytes - for (int i = 0; i < 16 && i < sizeof(hls_unified_thread_ctx_t); i++) { - if (ptr[i] != 0) { - all_zeros = false; - break; - } - } - - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - - if (all_zeros) { - // Memory appears to be zeroed out, which might indicate it's been freed - appears_invalid = true; - } - - if (appears_invalid) { - log_warn("Context %p appears to be invalid or already freed", ctx); - result = true; - } - } - + // Never probe memory through a pointer that may already have been freed. return result; } @@ -632,79 +573,11 @@ static void safe_cleanup_resources(AVFormatContext **input_ctx, AVPacket **pkt, } } - // Clean up HLS writer with safety checks if (writer) { - // CRITICAL FIX: Check if the pointer to pointer is valid before dereferencing - hls_writer_t *writer_to_free = NULL; - - // CRITICAL FIX: Add additional validation of the writer pointer - if (!*writer) { - log_debug("Writer is already NULL, nothing to clean up"); - } else { - // CRITICAL FIX: Use atomic pointer exchange to safely get and clear the writer pointer - // This ensures that no other thread can access the writer after we've taken ownership of it - writer_to_free = __atomic_exchange_n(writer, NULL, __ATOMIC_SEQ_CST); - - // CRITICAL FIX: Validate the writer pointer before using it - if (!writer_to_free) { - log_warn("Writer became NULL between checks"); - } else { - // CRITICAL FIX: Validate the writer structure before freeing - // This helps catch cases where the memory has been corrupted - bool writer_valid = true; - - // Basic validation of writer structure - if (writer_to_free->stream_name == NULL) { - log_warn("Writer has NULL stream_name, may be corrupted"); - writer_valid = false; - } - - if (writer_valid) { - // Get a copy of the stream name for logging - char writer_stream_name[MAX_STREAM_NAME]; - if (writer_to_free->stream_name) { - safe_strcpy(writer_stream_name, writer_to_free->stream_name, MAX_STREAM_NAME, 0); - } else { - safe_strcpy(writer_stream_name, "unknown", MAX_STREAM_NAME, 0); - } - - log_debug("Preparing to close HLS writer for stream %s", writer_stream_name); - - // Clear the pointer first to prevent double-free - *writer = NULL; - - // CRITICAL FIX: Add memory barrier to ensure memory operations are completed - __sync_synchronize(); - - // Safely free the HLS writer - log_debug("Safely closing HLS writer during cleanup for stream %s", writer_stream_name); - - // CRITICAL FIX: Add memory barrier before closing to ensure all accesses are complete - __sync_synchronize(); - - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(15); // 15 second timeout for writer close - - // Close the writer with additional protection - hls_writer_close(writer_to_free); - - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - - log_debug("Successfully closed HLS writer for stream %s", writer_stream_name); - } else { - log_warn("Skipping cleanup of invalid writer"); - *writer = NULL; // Still clear the pointer to prevent future access - } - } + hls_writer_t *writer_to_free = + __atomic_exchange_n(writer, NULL, __ATOMIC_SEQ_CST); + if (writer_to_free) { + hls_writer_close(writer_to_free); } } @@ -1807,25 +1680,12 @@ void *hls_unified_thread_func(void *arg) { // CRITICAL FIX: Only access writer if context is still valid hls_writer_t *writer_to_cleanup = NULL; if (context_valid_for_exit) { - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - // Safely check if writer exists and clean it up if (ctx_for_exit->writer) { log_warn("Writer for stream %s still exists after loop exit, cleaning up now", stream_name); writer_to_cleanup = __atomic_exchange_n(&ctx_for_exit->writer, NULL, __ATOMIC_SEQ_CST); } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); } else { log_warn("Context for stream %s is no longer valid, skipping writer cleanup", stream_name); } @@ -1842,16 +1702,6 @@ void *hls_unified_thread_func(void *arg) { // Only access ctx members if the context is not already freed if (context_valid_for_exit) { - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - // CRITICAL FIX: Check if context is still valid before accessing its members // This prevents use-after-free errors when the context has been freed if (!is_context_already_freed(ctx_for_exit) && !is_context_pending_deletion(ctx_for_exit)) { @@ -1868,9 +1718,6 @@ void *hls_unified_thread_func(void *arg) { log_info("Updated component state to STOPPED for stream %s after loop exit", stream_name); - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); } } @@ -1967,27 +1814,12 @@ void *hls_unified_thread_func(void *arg) { if (writer_to_cleanup) { log_debug("Writer is valid for stream %s, cleaning up", stream_name); - // Clean up the writer with additional try/catch-like protection log_info("Closing HLS writer for stream %s", stream_name); - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(15); // 15 second timeout for writer close - - // Close the writer with additional protection + // Close the writer hls_writer_close(writer_to_cleanup); writer_to_cleanup = NULL; - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - log_info("Successfully closed HLS writer for stream %s", stream_name); } else { log_debug("No writer to clean up for stream %s", stream_name); @@ -2059,16 +1891,6 @@ void *hls_unified_thread_func(void *arg) { // Free the context log_info("Freeing context for stream %s", stream_name); - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(15); // 15 second timeout for context free - // Mark the context as pending deletion to signal the thread mark_context_pending_deletion(ctx_to_free); @@ -2084,10 +1906,6 @@ void *hls_unified_thread_func(void *arg) { // Free the context with additional protection hls_guarded_free(ctx_to_free); - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - log_info("Successfully freed context for stream %s", stream_name); } } else { @@ -2104,16 +1922,6 @@ void *hls_unified_thread_func(void *arg) { // This is a comprehensive cleanup to ensure no FFmpeg resources are leaked // CRITICAL FIX: Added safety checks to prevent segfaults during shutdown - // Use try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - if (input_ctx) { log_warn("Input context still exists at thread exit for stream %s, forcing cleanup", stream_name); @@ -2140,10 +1948,6 @@ void *hls_unified_thread_func(void *arg) { } } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - // MEMORY LEAK FIX: Force FFmpeg to release any cached memory // This is a more aggressive approach to ensure all memory is freed // We call this directly here to ensure it happens before the thread exits @@ -2648,16 +2452,6 @@ int stop_hls_unified_stream(const char *stream_name) { // CRITICAL FIX: Clear the writer reference in the context before freeing to prevent double free if (ctx && !is_context_already_freed(ctx)) { - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - // Store a local copy of the stream name for logging char writer_stream_name[MAX_STREAM_NAME]; safe_strcpy(writer_stream_name, stream_name, sizeof(writer_stream_name), 0); // Use the stream_name we already have @@ -2669,10 +2463,6 @@ int stop_hls_unified_stream(const char *stream_name) { log_info("Clearing writer reference in context for stream %s", writer_stream_name); } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - // CRITICAL FIX: Safely close the writer if needed if (writer_to_cleanup) { // We don't free the writer here - the thread should handle that @@ -2739,16 +2529,6 @@ int stop_hls_unified_stream(const char *stream_name) { // Free the context log_info("Freeing context for stream %s", stream_name); - // Use a try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(15); // 15 second timeout for context free - // CRITICAL FIX: Mark the context as pending deletion to signal the thread mark_context_pending_deletion(ctx_to_free); @@ -2771,10 +2551,6 @@ int stop_hls_unified_stream(const char *stream_name) { // Free the context with additional protection hls_guarded_free(ctx_to_free); - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - log_info("Successfully freed context for stream %s", stream_name); } } else { @@ -2881,19 +2657,8 @@ static void ffmpeg_buffer_cleanup(void) { log_info("Forcing FFmpeg to release cached memory"); - // CRITICAL FIX: Use try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(1); // 1 second timeout - // Force garbage collection in FFmpeg - safely // This will help release any memory that might be cached - // CRITICAL FIX: Wrap in try/catch to prevent segfaults log_debug("Calling av_freep(NULL) to release cached memory"); // CRITICAL FIX: Skip direct av_freep(NULL) call during shutdown or hard deletion @@ -2979,10 +2744,6 @@ static void ffmpeg_buffer_cleanup(void) { log_info("Skipping complex FFmpeg allocations during shutdown to prevent crashes"); } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - log_info("FFmpeg memory cleanup completed"); in_cleanup = 0; } @@ -3066,16 +2827,6 @@ void cleanup_hls_unified_thread_system(void) { // Stop the watchdog thread stop_hls_watchdog(); - // CRITICAL FIX: Use try/catch-like approach with signal handling to prevent crashes - struct sigaction sa_old, sa_new; - sigaction(SIGSEGV, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore segmentation fault signal - sigaction(SIGSEGV, &sa_new, NULL); - - // Set alarm to prevent hanging if memory is inaccessible - alarm(2); // 2 second timeout - // MEMORY LEAK FIX: Force cleanup of any remaining FFmpeg resources log_info("Performing final FFmpeg resource cleanup during system shutdown"); @@ -3107,10 +2858,6 @@ void cleanup_hls_unified_thread_system(void) { // Clean up the freed contexts tracking system cleanup_freed_contexts_tracking(); - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGSEGV, &sa_old, NULL); - log_info("HLS unified thread system cleaned up"); in_system_cleanup = 0; } diff --git a/src/video/hls_writer.c b/src/video/hls_writer.c index 75674467..6cfa2352 100644 --- a/src/video/hls_writer.c +++ b/src/video/hls_writer.c @@ -12,7 +12,6 @@ #include #include // For O_NONBLOCK #include // For error codes -#include // For alarm #include #include #include @@ -961,24 +960,9 @@ void hls_writer_close(hls_writer_t *writer) { // Only proceed with trailer write if context is fully validated if (context_valid) { - // Set up a timeout for the trailer write operation - // Use sigaction for more reliable signal handling - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(5); // 5 second timeout for trailer write - // Use a safer approach to write the trailer int ret = av_write_trailer(local_output_ctx); - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - if (ret < 0) { recording_io_report_failure(RECORDING_IO_RESOURCE_HLS, RECORDING_IO_OPERATION_TRAILER, @@ -1001,16 +985,6 @@ void hls_writer_close(hls_writer_t *writer) { AVIOContext *pb_to_close = local_output_ctx->pb; local_output_ctx->pb = NULL; - // Set up a timeout for the AVIO close operation with proper signal handling - struct sigaction sa_old, sa_new; - sigaction(SIGALRM, NULL, &sa_old); - sa_new = sa_old; - sa_new.sa_handler = SIG_IGN; // Ignore alarm signal - sigaction(SIGALRM, &sa_new, NULL); - - // Set alarm - alarm(5); // 5 second timeout for AVIO close - // Close the AVIO context int close_ret = avio_closep(&pb_to_close); // Use safer avio_closep and pass the correct pointer if (close_ret < 0) { @@ -1019,10 +993,6 @@ void hls_writer_close(hls_writer_t *writer) { close_ret); } - // Cancel the alarm and restore signal handler - alarm(0); - sigaction(SIGALRM, &sa_old, NULL); - log_info("Successfully closed AVIO context for HLS writer for stream %s", stream_name); } diff --git a/src/video/onvif_detection.c b/src/video/onvif_detection.c index 00251275..48f1a764 100644 --- a/src/video/onvif_detection.c +++ b/src/video/onvif_detection.c @@ -151,6 +151,8 @@ static char *create_onvif_request(const char *username, const char *password, } } + if (!security_header) return NULL; + /* Build a WS-Addressing fragment if an action was provided. Using * `urn:uuid:` for MessageID and leaving mustUnderstand off on To/Action * — cameras that ignore WS-Addressing shouldn't then fault because they @@ -159,17 +161,29 @@ static char *create_onvif_request(const char *username, const char *password, generate_uuid(uuid, sizeof(uuid)); const char *safe_action = action ? action : ""; - const char *safe_to = to ? to : ""; + ezxml_t destination = ezxml_new("wsa:To"); + if (!destination) { + free(security_header); + return NULL; + } + ezxml_set_txt(destination, to ? to : ""); + char *destination_xml = ezxml_toxml(destination); + ezxml_free(destination); + if (!destination_xml) { + free(security_header); + return NULL; + } /* The WS-Addressing block inflates the envelope; reserve enough for * MessageID (urn:uuid:… = ~45 chars), the action URI, the destination URL, * the security header, and the body. 2 KB of slack above those sizes is * plenty. */ size_t envelope_size = strlen(request_body) + strlen(security_header) - + strlen(safe_action) + strlen(safe_to) + 2048; + + strlen(safe_action) + strlen(destination_xml) + 2048; char *soap_request = malloc(envelope_size); if (!soap_request) { free(security_header); + free(destination_xml); return NULL; } @@ -179,16 +193,17 @@ static char *create_onvif_request(const char *username, const char *password, "xmlns:wsa=\"http://www.w3.org/2005/08/addressing\">" "" "urn:uuid:%s" - "%s" + "%s" "%s" "%s" "" "%s" "", - uuid, safe_to, safe_action, security_header, request_body); + uuid, destination_xml, safe_action, security_header, request_body); free(security_header); + free(destination_xml); return soap_request; } @@ -241,16 +256,19 @@ static char *send_onvif_request_to_url(const char *full_url, const char *usernam curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, headers); // Set up response buffer - memory_struct_t chunk; - chunk.memory = malloc(1); - chunk.size = 0; + memory_struct_t chunk = {0}; curl_easy_setopt(curl_handle, CURLOPT_WRITEFUNCTION, write_memory_callback); curl_easy_setopt(curl_handle, CURLOPT_WRITEDATA, (void *)&chunk); - curl_easy_setopt(curl_handle, CURLOPT_TIMEOUT, 10); + curl_easy_setopt(curl_handle, CURLOPT_TIMEOUT, 10L); + curl_easy_setopt(curl_handle, CURLOPT_NOSIGNAL, 1L); // Perform request CURLcode res = curl_easy_perform(curl_handle); + long http_code = 0; + double elapsed = 0; + curl_easy_getinfo(curl_handle, CURLINFO_RESPONSE_CODE, &http_code); + curl_easy_getinfo(curl_handle, CURLINFO_TOTAL_TIME, &elapsed); // Clean up request free(soap_request); @@ -258,18 +276,17 @@ static char *send_onvif_request_to_url(const char *full_url, const char *usernam // Check for errors if (res != CURLE_OK) { - log_error("ONVIF Detection: curl_easy_perform() failed: %s", curl_easy_strerror(res)); + log_error("ONVIF request %s failed (curl=%d, HTTP=%ld, elapsed=%.3fs): %s", + action ? action : "unknown", (int)res, http_code, elapsed, + curl_easy_strerror(res)); free(chunk.memory); pthread_mutex_unlock(&curl_mutex); return NULL; } - // Get HTTP response code - long http_code = 0; - curl_easy_getinfo(curl_handle, CURLINFO_RESPONSE_CODE, &http_code); - - if (http_code != 200) { - log_error("ONVIF request to %s failed with HTTP code %ld", full_url, http_code); + if (http_code != 200 || chunk.size == 0) { + log_error("ONVIF request %s failed (HTTP=%ld, response=%zu bytes, elapsed=%.3fs)", + action ? action : "unknown", http_code, chunk.size, elapsed); if (chunk.size > 0) { onvif_log_soap_fault(chunk.memory, chunk.size, "ONVIF Detection"); } @@ -302,37 +319,6 @@ static char *send_onvif_request(const char *url, const char *username, const cha return send_onvif_request_to_url(full_url, username, password, request_body, action); } -// Extract subscription address from response -static char *extract_subscription_address(const char *response) { - if (!response) return NULL; - - // Try different namespace prefixes - const char *patterns[] = { - "", "", - "", "", - "
", "
" - }; - - for (int i = 0; i < 3; i++) { - const char *open_tag = patterns[(ptrdiff_t)i * 2]; - const char *close_tag = patterns[(ptrdiff_t)i * 2 + 1]; - const char *start = strstr(response, open_tag); - - if (start) { - size_t open_tag_len = strlen(open_tag); - const char *content_start = start + open_tag_len; - const char *end = strstr(content_start, close_tag); - - if (end) { - size_t length = (size_t)(end - content_start); - return strndup(content_start, length); - } - } - } - - return NULL; -} - /* Return the local name of an ezxml element (strips any "prefix:" prefix). */ static const char *local_name(ezxml_t el) { if (!el || !el->name) return ""; @@ -349,6 +335,28 @@ static ezxml_t child_by_local_name(ezxml_t parent, const char *local) { return NULL; } +/* The destination belongs to SubscriptionReference, not an unrelated + * WS-Addressing header. XML parsing also handles vendor prefixes, attributes, + * and escaped query-string separators in the returned URL. */ +static char *extract_subscription_address(const char *response) { + if (!response) return NULL; + char *copy = strdup(response); + if (!copy) return NULL; + ezxml_t root = ezxml_parse_str(copy, strlen(copy)); + char *address = NULL; + if (root && !ezxml_error(root)[0]) { + ezxml_t body = child_by_local_name(root, "Body"); + ezxml_t reply = child_by_local_name(body, "CreatePullPointSubscriptionResponse"); + ezxml_t reference = child_by_local_name(reply, "SubscriptionReference"); + ezxml_t destination = child_by_local_name(reference, "Address"); + if (destination && destination->txt && destination->txt[0]) + address = strdup(destination->txt); + } + if (root) ezxml_free(root); + free(copy); + return address; +} + /* Find the first descendant with the given local name. */ static ezxml_t descendant_by_local_name(ezxml_t node, const char *local) { if (!node || !local) return NULL; diff --git a/tests/database/benchmark_backup_verification.py b/tests/database/benchmark_backup_verification.py new file mode 100644 index 00000000..fa6f987b --- /dev/null +++ b/tests/database/benchmark_backup_verification.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +"""Compare integrity_check I/O with and without repeated whole-file eviction. + +Linux-only diagnostic, not a timing assertion for CI. Pass a directory on the +filesystem being measured (avoid tmpfs). Creates and removes its own fixture. +""" + +import argparse +import ast +import json +import os +from pathlib import Path +import re +import sqlite3 +import tempfile +import time + + +def read_bytes(): + values = dict(line.split(": ") for line in Path("/proc/self/io").read_text().splitlines()) + return int(values["read_bytes"]) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("directory", type=Path) + parser.add_argument("--rows", type=int, default=100000) + args = parser.parse_args() + root = Path(__file__).resolve().parents[2] + source = (root / "include/database/db_embedded_migrations.h").read_text() + migration = source.split("static const char migration_0055_up[] =", 1)[1] + migration = migration.split("static const char migration_0055_down", 1)[0] + schema = "".join(ast.literal_eval(part) for part in re.findall(r'"(?:[^"\\]|\\.)*"', migration)) + + with tempfile.TemporaryDirectory(prefix="backup-verification-", dir=args.directory) as folder: + path = Path(folder) / "audit.db" + with sqlite3.connect(path) as db: + db.executescript("CREATE TABLE users(id INTEGER PRIMARY KEY);" + "CREATE TABLE system_settings(key TEXT PRIMARY KEY,value TEXT);" + schema) + db.executemany( + "INSERT INTO audit_events(uuid,occurred_at,request_id,principal_user_id," + "principal_username,auth_method,action,target_type,target_uuid,outcome," + "remote_address,details_json) VALUES(?,?,?,?,?,?,?,?,?,?,?,?)", + ((f"event-{i:032d}", 1700000000 + i, f"request-{i}", i % 4, + "admin", "session", "recordings.list", "camera", f"camera-{i % 16}", + "success", "127.0.0.1", json.dumps({"detail": "x" * 400})) + for i in range(args.rows)), + ) + db.close() + print(json.dumps({"rows": args.rows, "file_bytes": path.stat().st_size})) + fd = os.open(path, os.O_RDONLY) + try: + # Both runs begin cold, outside the measurement interval. + for evict in (True, False): + os.fsync(fd) + os.posix_fadvise(fd, 0, 0, os.POSIX_FADV_DONTNEED) + callbacks = 0 + + def progress(): + nonlocal callbacks + callbacks += 1 + if evict: + os.posix_fadvise(fd, 0, 0, os.POSIX_FADV_DONTNEED) + return 0 + + db = sqlite3.connect(f"file:{path}?mode=ro", uri=True) + db.set_progress_handler(progress, 100000) + before = read_bytes() + started = time.monotonic() + result = db.execute("PRAGMA integrity_check").fetchall() + elapsed = time.monotonic() - started + fetched = read_bytes() - before + db.close() + print(json.dumps(dict(evict_each_callback=evict, seconds=round(elapsed, 3), + read_bytes=fetched, callbacks=callbacks, integrity=result))) + assert result == [("ok",)] + finally: + os.close(fd) + + +if __name__ == "__main__": + main() diff --git a/tests/database/db_backup_test.c b/tests/database/db_backup_test.c index 64c0fa31..8d7771e5 100644 --- a/tests/database/db_backup_test.c +++ b/tests/database/db_backup_test.c @@ -871,12 +871,53 @@ static void remove_test_db_and_backups(const char *db_path) { } } -// Regression test: shutdown_database() used to unconditionally take a full -// backup-and-verify snapshot on every clean shutdown, no matter how recently -// the hourly scheduled backup had already run -- on a multi-gigabyte -// production database this made every restart take minutes just to -// re-capture a few minutes of additional changes. Verifies that a shutdown -// happening shortly after a scheduled backup skips the redundant one. +// A failed scheduled attempt must wait for the next interval, but must not +// count as a successful snapshot when deciding whether to back up at shutdown. +static int test_failed_scheduled_backup_waits_without_suppressing_shutdown_backup(void) { + shutdown_database(); + remove_test_db_and_backups(TEST_SHUTDOWN_DB_PATH); + // Simulate an existing database with no successful backup yet. + sqlite3 *existing = NULL; + if (sqlite3_open(TEST_SHUTDOWN_DB_PATH, &existing) != SQLITE_OK) return -1; + sqlite3_exec(existing, "CREATE TABLE existing_fixture(id INTEGER);", NULL, NULL, NULL); + sqlite3_close(existing); + if (init_database(TEST_SHUTDOWN_DB_PATH) != 0) return -1; + g_config.db_backup_interval_minutes = 60; + + // An indexed fixture large enough to invoke the verification callback + // deterministically reaches the expired deadline without sleeping. + int rc = sqlite3_exec(get_db_handle(), + "CREATE TABLE retry_fixture(id INTEGER PRIMARY KEY, value TEXT);" + "WITH RECURSIVE n(i) AS (VALUES(1) UNION ALL SELECT i+1 FROM n WHERE i<20000) " + "INSERT INTO retry_fixture SELECT i,printf('value-%d',i) FROM n;" + "CREATE INDEX retry_value ON retry_fixture(value);", + NULL, NULL, NULL); + if (rc != SQLITE_OK) return -1; + + db_backup_set_max_duration_seconds_for_testing(-60); + int first = maybe_run_scheduled_database_backup(); + int second = maybe_run_scheduled_database_backup(); + db_backup_set_max_duration_seconds_for_testing(DB_BACKUP_MAX_DURATION_SECONDS_DEFAULT); + if (first != -1 || second != 0 || + count_timestamped_backups(TEST_SHUTDOWN_DB_PATH) != 0) { + printf("Failed backup was retried immediately or published as successful\n"); + shutdown_database(); + remove_test_db_and_backups(TEST_SHUTDOWN_DB_PATH); + return -1; + } + + shutdown_database(); + int count = count_timestamped_backups(TEST_SHUTDOWN_DB_PATH); + remove_test_db_and_backups(TEST_SHUTDOWN_DB_PATH); + if (count != 1) { + printf("Failed scheduled attempt incorrectly suppressed shutdown backup\n"); + return -1; + } + return 0; +} + +// A shutdown shortly after a successful scheduled backup should not repeat +// the full copy and verification of a potentially multi-gigabyte database. static int test_shutdown_skips_backup_when_recent_backup_exists(void) { int result = -1; @@ -1057,6 +1098,11 @@ int main(void) { return 1; } + if (test_failed_scheduled_backup_waits_without_suppressing_shutdown_backup() != 0) { + printf("Test failed: failed scheduled backup cooldown\n"); + return 1; + } + if (test_shutdown_skips_backup_when_recent_backup_exists() != 0) { printf("Test failed: shutdown did not skip a redundant backup\n"); return 1; diff --git a/tests/integration/specs/investigation.ui.spec.ts b/tests/integration/specs/investigation.ui.spec.ts index aadc87ad..5ac9df6f 100644 --- a/tests/integration/specs/investigation.ui.spec.ts +++ b/tests/integration/specs/investigation.ui.spec.ts @@ -1,6 +1,7 @@ import { test, expect } from '@playwright/test'; import { USERS, login } from '../fixtures/test-fixtures'; +import { serveRecordingMedia } from '../fixtures/recording-media'; /* One solid-blue 320x240 VP8 frame. Keeping the fixture inline makes the * metadata transition deterministic without depending on a camera or ffmpeg. */ @@ -16,9 +17,6 @@ const END_TIME = START_TIME + 60; test.describe('Investigation player regressions @ui @investigation', () => { test.beforeEach(async ({ page }) => { await login(page, USERS.admin); - }); - - test('keeps controls visible and preserves a 4:3 recording after metadata loads', async ({ page }) => { await page.route('**/api/streams?**', route => route.fulfill({ json: { streams: [{ @@ -78,6 +76,9 @@ test.describe('Investigation player regressions @ui @investigation', () => { body: FOUR_BY_THREE_WEBM, })); + }); + + test('keeps controls visible and preserves a 4:3 recording after metadata loads', async ({ page }) => { await page.goto( `/investigation.html?cameras=${CAMERA_UUID}&start=${START_TIME}` + `&end=${END_TIME}&cursor=${START_TIME + 1}`, @@ -114,4 +115,39 @@ test.describe('Investigation player regressions @ui @investigation', () => { await expect(controls.getByRole('button', { name: 'Fullscreen' })).toBeVisible(); await expect(page.getByTestId('fisheye-eptz-canvas')).toHaveCount(0); }); + + test('native play and pause synchronize shared controls without stopping on media reload', async ({ page }) => { + await page.route('**/api/recordings/play/568*', serveRecordingMedia); + await page.goto( + `/investigation.html?cameras=${CAMERA_UUID}&start=${START_TIME}` + + `&end=${END_TIME}&cursor=${START_TIME + 1}`, + { waitUntil: 'domcontentloaded' }, + ); + const player = page.locator('.investigation-player').first(); + const video = player.locator('video'); + const sharedPlay = page.locator('.investigation-play-button'); + const localControls = player.locator('.investigation-player-controls'); + await expect.poll(() => video.evaluate(el => el.readyState)).toBeGreaterThanOrEqual(2); + + // These are real media events, as emitted by the browser's native controls. + await video.evaluate(el => el.play()); + await expect(sharedPlay).toContainText('Pause'); + await expect(localControls.getByRole('button', { name: 'Pause' })).toBeVisible(); + await video.evaluate(el => el.pause()); + await expect(sharedPlay).toContainText('Play'); + await expect(localControls.getByRole('button', { name: 'Play', exact: true })).toBeVisible(); + + await sharedPlay.click(); + await expect.poll(() => video.evaluate(el => el.paused)).toBe(false); + // load() emits pause while resetting the media element. It must not pause + // the shared investigation clock or other cameras. + await video.evaluate(el => el.load()); + await expect.poll(() => video.evaluate(el => el.readyState)).toBeGreaterThanOrEqual(2); + await expect.poll(() => video.evaluate(el => el.paused)).toBe(false); + await expect(sharedPlay).toContainText('Pause'); + + await video.evaluate(el => el.pause()); + await expect(sharedPlay).toContainText('Play'); + }); + }); diff --git a/tests/integration/synthetic-onvif-prebuffer.py b/tests/integration/synthetic-onvif-prebuffer.py new file mode 100644 index 00000000..a9244c5f --- /dev/null +++ b/tests/integration/synthetic-onvif-prebuffer.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Verify actual pre-trigger footage using loopback RTSP and simulated ONVIF. + +Requires a built lightnvr, go2rtc, ffmpeg and ffprobe. Runs for about 45 seconds. +All processes and files belong to a new /tmp/lightnvr-prebuffer-* directory; +logs, MP4 footage and result.json are retained for inspection. +""" + +import argparse +import base64 +import json +import os +from pathlib import Path +import signal +import socket +import sqlite3 +import subprocess +import tempfile +import threading +import time +import urllib.request +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + + +ROOT = Path(__file__).resolve().parents[2] + + +def unused_port(): + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def wait_for(check, timeout=20): + deadline = time.monotonic() + timeout + last_error = None + while time.monotonic() < deadline: + try: + value = check() + if value: + return value + except (OSError, ValueError, sqlite3.Error) as exc: + # Services, progress files and database rows may not be ready yet. + # Retry transient failures, retaining the last one for diagnosis. + last_error = exc + time.sleep(0.2) + message = f"Timed out after {timeout}s: {check}" + if last_error is not None: + message += f"; last transient error: {last_error}" + raise RuntimeError(message) from last_error + + +class Camera(BaseHTTPRequestHandler): + motion = threading.Event() + + def log_message(self, *args): + pass + + def do_POST(self): + body = self.rfile.read(int(self.headers.get("Content-Length", 0))).decode() + base = f"http://127.0.0.1:{self.server.server_port}" + if "GetServices" in body: + reply = ( + "" + "http://www.onvif.org/ver10/events/wsdl" + f"{base}/events" + ) + elif "CreatePullPointSubscription" in body: + reply = ( + "" + f"{base}/pull" + "2026-09-12T12:00:00Z" + "2026-09-12T12:10:00Z" + "" + ) + elif "PullMessages" in body: + time.sleep(0.2) + reply = "" + if self.motion.is_set(): + reply += ( + "" + "tns1:RuleEngine/CellMotionDetector/Motion" + "" + '' + "" + ) + reply += "" + else: + reply = "" + payload = ( + '' + f"{reply}" + ).encode() + self.send_response(200) + self.send_header("Content-Type", "application/soap+xml") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--lightnvr", type=Path, default=ROOT / "build/bin/lightnvr") + parser.add_argument("--go2rtc", type=Path, default=ROOT / "go2rtc/go2rtc") + args = parser.parse_args() + for binary in (args.lightnvr, args.go2rtc): + if not binary.is_file(): + parser.error(f"Missing binary: {binary}") + + run = Path(tempfile.mkdtemp(prefix="lightnvr-prebuffer-")) + print(f"Artifacts: {run}", flush=True) + api, rtsp, web = unused_port(), unused_port(), unused_port() + camera = ThreadingHTTPServer(("127.0.0.1", 0), Camera) + threading.Thread(target=camera.serve_forever, daemon=True).start() + processes = [] + + def start(command, label): + # Popen gives the child its own descriptor; close the parent's copy + # immediately, including when spawning the child fails. + with open(run / f"{label}.log", "wb") as log: + process = subprocess.Popen(command, stdout=log, stderr=subprocess.STDOUT, + start_new_session=True, cwd=ROOT) + processes.append(process) + + def request(path, data=None): + headers = {"Authorization": "Basic " + base64.b64encode(b"admin:admin").decode()} + if data is not None: + headers["Content-Type"] = "application/json" + req = urllib.request.Request( + f"http://127.0.0.1:{web}{path}", headers=headers, + data=json.dumps(data).encode() if data is not None else None, + ) + with urllib.request.urlopen(req, timeout=20) as response: + return json.load(response) + + def source_seconds(): + progress = run / "progress" + rows = progress.read_text().splitlines() if progress.exists() else [] + values = [int(row.split("=")[1]) / 1e6 for row in rows + if row.startswith("out_time_us=") and "N/A" not in row] + return values[-1] if values else 0 + + def recordings(): + with sqlite3.connect(f"file:{run}/lightnvr.db?mode=ro", uri=True) as db: + db.row_factory = sqlite3.Row + return [dict(row) for row in db.execute( + "SELECT id,file_path,start_time,end_time,is_complete FROM recordings " + "WHERE stream_name='prebuffer' ORDER BY id" + )] + + def sample_color(media, offset): + return list(subprocess.check_output([ + "ffmpeg", "-v", "error", "-ss", str(offset), "-i", media, + "-frames:v", "1", "-vf", "scale=1:1", "-pix_fmt", "rgb24", + "-f", "rawvideo", "pipe:1", + ], timeout=15)) + + try: + (run / "go2rtc.yaml").write_text( + f'api:\n listen: "127.0.0.1:{api}"\n' + f'rtsp:\n listen: "127.0.0.1:{rtsp}"\n' + 'webrtc:\n listen: ""\nstreams:\n fixture: []\n' + ) + config = (ROOT / "config/lightnvr-test.ini").read_text() + config = config.replace("/tmp/lightnvr-test", str(run)) + config = config.replace("18080", str(web)).replace("./web/dist", str(ROOT / "web/dist")) + # The separate loopback go2rtc process supplies RTSP; lightnvr reads it directly. + config = config.replace("enabled = true\nbinary_path", "enabled = false\nbinary_path") + (run / "test.ini").write_text(config) + for subdir in ("recordings/mp4", "models"): + (run / subdir).mkdir(parents=True, exist_ok=True) + start([str(args.go2rtc.resolve()), "-c", str(run / "go2rtc.yaml")], "go2rtc") + + def go2rtc_ready(): + with urllib.request.urlopen(f"http://127.0.0.1:{api}/api", timeout=1) as response: + return response.status == 200 + + wait_for(go2rtc_ready) + start([str(args.lightnvr.resolve()), "-c", str(run / "test.ini")], "application") + wait_for(lambda: request("/api/system")) + # Red before source t=12, blue thereafter. A trigger at t>=15 can only + # produce a red first frame if the recording includes pre-trigger media. + start([ + "ffmpeg", "-hide_banner", "-loglevel", "warning", "-re", "-f", "lavfi", + "-i", "color=c=red:s=320x240:r=10", "-vf", + "drawbox=color=blue:t=fill:enable='gte(t,12)'", "-t", "65", "-an", + "-c:v", "libx264", "-preset", "ultrafast", "-tune", "zerolatency", + "-g", "10", "-keyint_min", "10", "-sc_threshold", "0", + "-progress", str(run / "progress"), "-f", "rtsp", "-rtsp_transport", "tcp", + f"rtsp://127.0.0.1:{rtsp}/fixture", + ], "source") + wait_for(lambda: source_seconds() > 0) + request("/api/streams", { + "name": "prebuffer", "url": f"rtsp://127.0.0.1:{rtsp}/fixture", + "enabled": True, "streaming_enabled": False, "record": False, + "record_audio": False, "detection_based_recording": True, + "detection_model": "onvif", "detection_threshold": 50, + "detection_interval": 1, "pre_detection_buffer": 8, + "post_detection_buffer": 3, "onvif_port": camera.server_port, + }) + wait_for(lambda: source_seconds() >= 15, timeout=25) + assert not recordings(), "Recording started before any simulated motion" + trigger = time.time() + Camera.motion.set() + print(f"Motion start: epoch={trigger}, source_seconds={source_seconds()}", flush=True) + wait_for(lambda: source_seconds() >= 18, timeout=10) + Camera.motion.clear() + print("Waiting for ONVIF hold, detection grace and post-buffer", flush=True) + rows = wait_for(lambda: [row for row in recordings() if row["is_complete"]], timeout=40) + assert len(rows) == 1, f"Expected one detection clip: {rows}" + row = rows[0] + media = row["file_path"] + probe = json.loads(subprocess.check_output( + ["ffprobe", "-v", "error", "-show_format", "-of", "json", media], timeout=15)) + duration = float(probe["format"]["duration"]) + first, last = sample_color(media, 0), sample_color(media, duration - 1) + result = dict(trigger_epoch=trigger, first_frame_rgb=first, last_frame_rgb=last, + duration_seconds=duration, recording=row) + (run / "result.json").write_text(json.dumps(result, indent=2) + "\n") + print(json.dumps(result, indent=2), flush=True) + assert len(first) == 3 and first[0] > 200 and first[2] < 50, "Missing red pre-trigger footage" + assert len(last) == 3 and last[2] > 200 and last[0] < 50, "Missing blue footage after trigger" + assert trigger - row["start_time"] >= 4, "Missing pre-trigger metadata interval" + assert duration >= 15, "Recorded media is too short" + finally: + for process in reversed(processes): + if process.poll() is None: + os.killpg(process.pid, signal.SIGTERM) + try: + process.wait(timeout=25) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + process.wait() + camera.shutdown() + camera.server_close() + + +if __name__ == "__main__": + main() diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 65f80809..981932de 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -248,6 +248,14 @@ endif() add_layer3_test(test_stream_manager) add_layer3_test(test_stream_state) add_layer3_test(test_packet_buffer) +add_layer3_test(test_hls_cleanup) +target_compile_definitions(test_hls_cleanup PRIVATE + TEST_RECORDING_PATH="${CMAKE_SOURCE_DIR}/tests/integration/fixtures/recording.mp4" +) +target_link_options(test_hls_cleanup PRIVATE + "-Wl,--wrap=alarm" "-Wl,--wrap=sigaction" "-Wl,--wrap=av_read_frame" +) +set_tests_properties(test_hls_cleanup PROPERTIES TIMEOUT 30) add_layer3_test(test_mp4_writer_watchdog) add_layer3_test(test_mp4_segment_timestamps) add_layer3_test(test_mp4_segment_boundaries) diff --git a/tests/unit/test_detection_system_onvif.c b/tests/unit/test_detection_system_onvif.c index 8acf9825..024b2fa2 100644 --- a/tests/unit/test_detection_system_onvif.c +++ b/tests/unit/test_detection_system_onvif.c @@ -46,6 +46,10 @@ typedef struct { bool reject_create; int granted_lease_seconds; int pull_failures_remaining; + int empty_pull_responses_remaining; + int dropped_pulls_remaining; + bool scoped_subscription_address; + bool saw_subscription_target; int renew_failures_remaining; int create_count; int pull_count; @@ -170,6 +174,17 @@ static void *fake_onvif_server_main(void *arg) { "%s" "", server->port, termination); + if (server->scoped_subscription_address) { + snprintf(body, sizeof(body), + "
http://127.0.0.1:%d/wrong" + "
" + "" + "http://127.0.0.1:%d/pull_service?one=1&two=2" + "2026-08-28T12:00:00Z" + "%s" + "", + server->port, server->port, termination); + } send_xml_response(client_fd, body); } else if (strstr(request, "renew_count++; @@ -194,6 +209,27 @@ static void *fake_onvif_server_main(void *arg) { ""); } else if (strstr(request, "PullMessages")) { server->pull_count++; + if (server->scoped_subscription_address) { + server->saw_subscription_target = + strstr(request, "POST /pull_service?one=1&two=2 HTTP/") != NULL && + strstr(request, "/pull_service?one=1&two=2") != NULL; + if (!server->saw_subscription_target) { + send_xml_response_with_status(client_fd, 400, "Bad Request", ""); + close(client_fd); + continue; + } + } + if (server->dropped_pulls_remaining > 0) { + server->dropped_pulls_remaining--; + close(client_fd); + continue; + } + if (server->empty_pull_responses_remaining > 0) { + server->empty_pull_responses_remaining--; + send_xml_response(client_fd, ""); + close(client_fd); + continue; + } if (server->pull_failures_remaining > 0) { server->pull_failures_remaining--; send_xml_response_with_status(client_fd, 503, "Unavailable", @@ -327,6 +363,50 @@ void test_onvif_transient_pull_failure_reuses_subscription(void) { TEST_ASSERT_EQUAL_INT(1, server.unsubscribe_count); } +// A dropped HTTP connection and an empty HTTP 200 are both request failures; +// neither should destroy a still-valid camera subscription after one poll. +static void check_empty_pull_recovery(bool drop_connection) { + fake_onvif_server_t server; + TEST_ASSERT_EQUAL_INT(0, start_fake_onvif_server(&server)); + server.dropped_pulls_remaining = drop_connection ? 1 : 0; + server.empty_pull_responses_remaining = drop_connection ? 0 : 1; + TEST_ASSERT_EQUAL_INT(0, init_detection_system()); + char url[64]; + snprintf(url, sizeof(url), "http://127.0.0.1:%d", server.port); + detection_result_t result = {0}; + int first = detect_motion_onvif(url, "", "", &result, ""); + int second = detect_motion_onvif(url, "", "", &result, ""); + shutdown_onvif_and_stop_server(&server); + TEST_ASSERT_EQUAL_INT(-1, first); + TEST_ASSERT_EQUAL_INT(0, second); + TEST_ASSERT_EQUAL_INT(1, server.create_count); + TEST_ASSERT_EQUAL_INT(2, server.pull_count); + TEST_ASSERT_EQUAL_INT(1, server.unsubscribe_count); +} + +void test_onvif_dropped_pull_reuses_subscription(void) { + check_empty_pull_recovery(true); +} + +void test_onvif_empty_http_success_is_a_failed_poll(void) { + check_empty_pull_recovery(false); +} + +void test_onvif_uses_scoped_subscription_address_with_escaped_query(void) { + fake_onvif_server_t server; + TEST_ASSERT_EQUAL_INT(0, start_fake_onvif_server(&server)); + server.scoped_subscription_address = true; + TEST_ASSERT_EQUAL_INT(0, init_detection_system()); + char url[64]; + snprintf(url, sizeof(url), "http://127.0.0.1:%d", server.port); + detection_result_t result = {0}; + int rc = detect_motion_onvif(url, "", "", &result, ""); + shutdown_onvif_and_stop_server(&server); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_TRUE(server.saw_subscription_target); + TEST_ASSERT_EQUAL_INT(1, server.create_count); +} + void test_onvif_discovered_endpoint_is_not_retried_as_fallback(void) { fake_onvif_server_t server; TEST_ASSERT_EQUAL_INT(0, start_fake_onvif_server(&server)); @@ -579,6 +659,9 @@ int main(void) { RUN_TEST(test_init_detection_system_initializes_onvif_detection); RUN_TEST(test_onvif_subscription_requests_tapo_lease); RUN_TEST(test_onvif_transient_pull_failure_reuses_subscription); + RUN_TEST(test_onvif_dropped_pull_reuses_subscription); + RUN_TEST(test_onvif_empty_http_success_is_a_failed_poll); + RUN_TEST(test_onvif_uses_scoped_subscription_address_with_escaped_query); RUN_TEST(test_onvif_discovered_endpoint_is_not_retried_as_fallback); RUN_TEST(test_onvif_subscription_renews_camera_granted_lease); RUN_TEST(test_onvif_failed_renew_unsubscribes_before_recreate); diff --git a/tests/unit/test_hls_cleanup.c b/tests/unit/test_hls_cleanup.c new file mode 100644 index 00000000..36b2557d --- /dev/null +++ b/tests/unit/test_hls_cleanup.c @@ -0,0 +1,221 @@ +#define _POSIX_C_SOURCE 200809L +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "unity.h" +#include "core/config.h" +#include "core/logger.h" +#include "core/shutdown_coordinator.h" +#include "video/hls_writer.h" +#include "video/hls/hls_unified_thread.h" +#include "video/stream_manager.h" +#include "video/stream_state.h" + +static atomic_int alarm_calls; +static atomic_int disposition_changes; +static char test_dir[128]; +static pthread_barrier_t close_barrier; +static atomic_bool pace_input; + +int __real_av_read_frame(AVFormatContext *input, AVPacket *packet); + +int __wrap_av_read_frame(AVFormatContext *input, AVPacket *packet) { + // Keep the local media source open long enough to stop a running thread, + // without requiring an external RTSP server in the unit-test suite. + if (atomic_load(&pace_input) && input->url && + strcmp(input->url, TEST_RECORDING_PATH) == 0) usleep(10000); + return __real_av_read_frame(input, packet); +} + +int __real_sigaction(int sig, const struct sigaction *action, + struct sigaction *previous); + +int __wrap_sigaction(int sig, const struct sigaction *action, + struct sigaction *previous) { + if (action && (sig == SIGALRM || sig == SIGSEGV)) + atomic_fetch_add(&disposition_changes, 1); + return __real_sigaction(sig, action, previous); +} + +unsigned int __wrap_alarm(unsigned int seconds) { + (void)seconds; + atomic_fetch_add(&alarm_calls, 1); + // Do not let a regression terminate the test runner with SIGALRM. + return 0; +} + +static void remove_test_files(const char *path) { + DIR *dir = opendir(path); + if (!dir) return; + struct dirent *entry; + while ((entry = readdir(dir))) { + if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, "..")) continue; + char child[512]; + snprintf(child, sizeof(child), "%s/%s", path, entry->d_name); + if (unlink(child) != 0) remove_test_files(child); + } + closedir(dir); + rmdir(path); +} + +void setUp(void) { + strcpy(test_dir, "/tmp/lightnvr_hls_cleanup_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(test_dir)); + load_default_config(&g_config); + snprintf(g_config.storage_path, sizeof(g_config.storage_path), "%s", test_dir); + g_config.max_streams = 4; + init_shutdown_coordinator(); + atomic_store(&alarm_calls, 0); + atomic_store(&disposition_changes, 0); +} + +void tearDown(void) { + cleanup_all_hls_writers(); + remove_test_files(test_dir); +} + +static void *close_writer(void *arg) { + pthread_barrier_wait(&close_barrier); + hls_writer_close(arg); + return NULL; +} + +void test_parallel_writer_teardown_and_recreation_preserves_signals(void) { + enum { CAMERAS = 3, CYCLES = 3 }; + for (int cycle = 0; cycle < CYCLES; ++cycle) { + hls_writer_t *writers[CAMERAS]; + pthread_t threads[CAMERAS]; + char names[CAMERAS][32]; + char playlists[CAMERAS][MAX_PATH_LENGTH + 32]; + for (int camera = 0; camera < CAMERAS; ++camera) { + snprintf(names[camera], sizeof(names[camera]), "cleanup-%d", camera); + writers[camera] = hls_writer_create(test_dir, names[camera], 2); + TEST_ASSERT_NOT_NULL(writers[camera]); + AVFormatContext *input = NULL; + TEST_ASSERT_EQUAL_INT(0, avformat_open_input( + &input, TEST_RECORDING_PATH, NULL, NULL)); + TEST_ASSERT_GREATER_OR_EQUAL(0, avformat_find_stream_info(input, NULL)); + TEST_ASSERT_EQUAL_INT(0, hls_writer_initialize(writers[camera], input->streams[0])); + AVPacket *packet = av_packet_alloc(); + TEST_ASSERT_NOT_NULL(packet); + for (int i = 0; i < 8; ++i) { + TEST_ASSERT_EQUAL_INT(0, av_read_frame(input, packet)); + TEST_ASSERT_GREATER_OR_EQUAL(0, hls_writer_write_packet( + writers[camera], packet, input->streams[0])); + av_packet_unref(packet); + } + av_packet_free(&packet); + avformat_close_input(&input); + snprintf(playlists[camera], sizeof(playlists[camera]), "%s/index.m3u8", + writers[camera]->output_dir); + } + TEST_ASSERT_EQUAL_INT(0, pthread_barrier_init(&close_barrier, NULL, CAMERAS)); + for (int camera = 0; camera < CAMERAS; ++camera) + TEST_ASSERT_EQUAL_INT(0, pthread_create(&threads[camera], NULL, + close_writer, writers[camera])); + for (int camera = 0; camera < CAMERAS; ++camera) { + TEST_ASSERT_EQUAL_INT(0, pthread_join(threads[camera], NULL)); + TEST_ASSERT_NULL(find_hls_writer_by_stream_name(names[camera])); + AVFormatContext *recorded = NULL; + TEST_ASSERT_EQUAL_INT(0, avformat_open_input( + &recorded, playlists[camera], NULL, NULL)); + AVPacket *packet = av_packet_alloc(); + TEST_ASSERT_NOT_NULL(packet); + TEST_ASSERT_EQUAL_INT(0, av_read_frame(recorded, packet)); + av_packet_free(&packet); + avformat_close_input(&recorded); + } + pthread_barrier_destroy(&close_barrier); + } + TEST_ASSERT_EQUAL_INT(0, atomic_load(&alarm_calls)); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&disposition_changes)); +} + +void test_unified_system_cleanup_preserves_signals(void) { + cleanup_hls_unified_thread_system(); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&alarm_calls)); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&disposition_changes)); +} + +typedef struct { + char name[32]; + bool restart; + int result; +} lifecycle_task_t; + +static void *change_stream_lifecycle(void *arg) { + lifecycle_task_t *task = arg; + pthread_barrier_wait(&close_barrier); + task->result = task->restart ? restart_hls_unified_stream(task->name) + : stop_hls_unified_stream(task->name); + return NULL; +} + +void test_parallel_unified_stream_shutdown_and_restart(void) { + enum { CAMERAS = 3 }; + lifecycle_task_t tasks[CAMERAS] = {0}; + pthread_t threads[CAMERAS]; + TEST_ASSERT_EQUAL_INT(0, init_stream_state_manager(4)); + TEST_ASSERT_EQUAL_INT(0, init_stream_manager(4)); + atomic_store(&pace_input, true); + for (int camera = 0; camera < CAMERAS; ++camera) { + snprintf(tasks[camera].name, sizeof(tasks[camera].name), "lifecycle-%d", camera); + stream_config_t config = {0}; + snprintf(config.name, sizeof(config.name), "%s", tasks[camera].name); + snprintf(config.url, sizeof(config.url), "%s", TEST_RECORDING_PATH); + config.enabled = true; + config.streaming_enabled = true; + config.segment_duration = 2; + config.protocol = STREAM_PROTOCOL_TCP; + TEST_ASSERT_NOT_NULL(add_stream(&config)); + TEST_ASSERT_EQUAL_INT(0, start_hls_unified_stream(config.name)); + } + for (int cycle = 0; cycle < 3; ++cycle) { + for (int camera = 0; camera < CAMERAS; ++camera) { + int tries = 0; + while (!is_hls_stream_active(tasks[camera].name) && tries++ < 100) + usleep(50000); + TEST_ASSERT_TRUE(is_hls_stream_active(tasks[camera].name)); + tasks[camera].restart = cycle < 2; + } + TEST_ASSERT_EQUAL_INT(0, pthread_barrier_init(&close_barrier, NULL, CAMERAS)); + for (int camera = 0; camera < CAMERAS; ++camera) + TEST_ASSERT_EQUAL_INT(0, pthread_create(&threads[camera], NULL, + change_stream_lifecycle, &tasks[camera])); + for (int camera = 0; camera < CAMERAS; ++camera) { + TEST_ASSERT_EQUAL_INT(0, pthread_join(threads[camera], NULL)); + TEST_ASSERT_EQUAL_INT(0, tasks[camera].result); + } + pthread_barrier_destroy(&close_barrier); + } + for (int camera = 0; camera < CAMERAS; ++camera) { + TEST_ASSERT_FALSE(is_hls_stream_active(tasks[camera].name)); + TEST_ASSERT_NULL(find_hls_writer_by_stream_name(tasks[camera].name)); + } + atomic_store(&pace_input, false); + shutdown_stream_manager(); + shutdown_stream_state_manager(); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&alarm_calls)); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&disposition_changes)); +} + +int main(void) { + init_logger(); + UNITY_BEGIN(); + RUN_TEST(test_parallel_writer_teardown_and_recreation_preserves_signals); + RUN_TEST(test_parallel_unified_stream_shutdown_and_restart); + RUN_TEST(test_unified_system_cleanup_preserves_signals); + int result = UNITY_END(); + shutdown_logger(); + return result; +} diff --git a/web/js/components/preact/investigation/InvestigationView.jsx b/web/js/components/preact/investigation/InvestigationView.jsx index dd28d2f4..24ba1586 100644 --- a/web/js/components/preact/investigation/InvestigationView.jsx +++ b/web/js/components/preact/investigation/InvestigationView.jsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'preact/hooks'; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'preact/hooks'; import { fetchJSON, useQuery } from '../../../query-client.js'; import { useI18n } from '../../../i18n.js'; @@ -83,6 +83,7 @@ function InvestigationPlayer({ const cursorRef = useRef(cursor); const playbackRef = useRef({ playing, speed }); playbackRef.current = { playing, speed }; + const mediaLoadingRef = useRef(true); const regionAnchorRef = useRef(null); const coverageSegment = findSegmentAt(track.segments, cursor); const [resolvedSegment, setResolvedSegment] = useState(null); @@ -212,6 +213,7 @@ function InvestigationPlayer({ useEffect(() => { const video = videoRef.current; if (!video) return undefined; + mediaLoadingRef.current = true; if (!segment) { video.pause(); video.removeAttribute('src'); @@ -222,6 +224,7 @@ function InvestigationPlayer({ setStatus('loading'); const loaded = () => { + mediaLoadingRef.current = false; if (video.videoWidth > 0 && video.videoHeight > 0) { setVideoDimensions({ width: video.videoWidth, height: video.videoHeight }); } @@ -240,6 +243,7 @@ function InvestigationPlayer({ }, }); return () => { + mediaLoadingRef.current = true; video.removeEventListener('loadedmetadata', loaded); cleanup(); }; @@ -249,17 +253,34 @@ function InvestigationPlayer({ seekToCursor(); }, [cursor, seekToCursor]); - useEffect(() => { + useLayoutEffect(() => { const video = videoRef.current; if (!video || !segment || video.readyState < 1) return; video.playbackRate = speed; - if (playing) { + if (playing && video.paused) { video.play().catch(() => setStatus('paused-by-browser')); - } else { + } else if (!playing && !video.paused) { video.pause(); } }, [playing, speed, segment?.id]); + const handleNativePlay = () => { + const video = videoRef.current; + if (video && !video.paused && !mediaLoadingRef.current) { + onPlayingChange(true); + } + }; + + const handleNativePause = () => { + const video = videoRef.current; + // Loading another segment and reaching its end also emit pause events. + // The shared clock must keep advancing through those transitions/gaps. + if (video?.paused && !video.ended && !video.error && + video.readyState >= 1 && !mediaLoadingRef.current) { + onPlayingChange(false); + } + }; + const toggleFullscreen = useCallback(async () => { const shell = videoShellRef.current; if (!shell) return; @@ -338,6 +359,8 @@ function InvestigationPlayer({ playsInline preload="metadata" controls={!eptzActive} + onPlay={handleNativePlay} + onPause={handleNativePause} onWaiting={() => setStatus('late')} onPlaying={() => setStatus('ready')} onCanPlay={() => setStatus('ready')}