* fix: free a stream's monitor and playback buffer when nph-zms exits
Every nph-zms exited with ~1 MB still allocated, reported by LeakSanitizer
on every stream:
- The monitor's zones each hold a shared_ptr back to it, so releasing the
stream's reference never freed the Monitor, its zones (a full-frame zone
image each), camera or links. zmc breaks the cycle with disconnect() on
exit; zms never did. ~StreamBase now calls it.
- The buffered-playback SwapImage array (connkey + buffer) was never
deleted. MonitorStream now frees it in its destructor.
Freed at process exit either way, so nothing grows, but it shows up in
every leak report and hides real ones.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: check a stream frees its monitor and zones on destruction refs #5186
A stream holds a monitor with one zone; once the stream is destroyed, a
weak_ptr to the monitor must have expired. Each Zone holds a shared_ptr
back to its Monitor, so without ~StreamBase breaking the cycle the monitor,
its zones and their images outlive the stream. Fails on 82901ac1f
(REQUIRE( watched.expired() )), passes with the fix. Needs no database or
sanitizer, so ci-cpp-tests runs it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor: hold the buffered-playback array in a unique_ptr refs #5186
temp_image_buffer is now a std::unique_ptr<SwapImage[]>, so it is freed
with the stream without a hand-written destructor, and the leak cannot
come back by dropping that line. The array is private and only allocated
in runStream(), so a unit test cannot see it freed without a sanitizer;
under LeakSanitizer, buffered nph-zms streams leaked 48,008 bytes from
runStream() on 82901ac1f and nothing with this branch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
openComms() logs an error but carries on to bind() when it cannot open or
flock() the .lock file, so a zms can be serving without holding the lock. In
that state the unlink in closeComms() could remove a socket belonging to the
zms that does hold the lock, leaving it unreachable.
Guard the unlink on lock_fd >= 0. Leaking the socket file when we never held
the lock is no worse than the behaviour before the unlink was added.
Expand the comment to spell out that this path is the command socket rather
than the .lock file, that our successor unlinks it itself on the way to bind(),
and why the unlink has to precede releasing the lock.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
closeComms() left /var/lib/zoneminder/sock/zms-NNNNNNs.sock behind, and the
only thing that ever removed it was the unlink() before bind() in a later zms
that happened to draw the same connkey. Socket files accumulated indefinitely.
web/ajax/stream.php uses file_exists() on that path to decide whether zms is
listening, and waits up to a second for it to appear before giving up. A file
left by an exited zms defeats that wait: the check passes immediately, the
sendto() gets ECONNREFUSED, and the command is reported as failed even though
the new zms was about to bind. genConnKey() draws from six digits, so a page
cycling monitors every five seconds reuses a key well within an hour.
Unlink while we still hold the flock. A second zms with the same connkey blocks
on flock(LOCK_EX) in openComms() before it unlinks and binds, so it cannot have
created its own socket yet and we can't delete a file belonging to it. The lock
file itself is still left alone, since another zms may be waiting on it.
This is best effort: zms killed by a signal still leaves the socket behind. In
that case the process is usually still running, so the file being there is not
wrong.
Also reset lock_fd after closing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The command processor thread could crash dereferencing monitor->shared_data
or monitor->trigger_data while the main thread was in loadMonitor() calling
disconnect() which munmaps shared memory and nulls those pointers.
Three fixes:
- Guard against null monitor at the top of processCommand
- Add monitor_mutex to StreamBase, held during disconnect/connect in
loadMonitor and during shared memory reads in processCommand, so the
command thread cannot access shared memory while it is being remapped
- Initialize status_data.analysing and status_data.score in the
!ShmValid branch where they were previously sent uninitialized
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change lock_fd error check from <= 0 to < 0 since open() returns -1
on error and 0 is a valid file descriptor
- Use -1 consistently as invalid fd marker
- Fix close check from > 0 to >= 0 to handle fd 0 correctly
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
zm_videostore.cpp:
- Fix incorrect size passed to av_stream_add_side_data (was sizeof(int32_t),
should be sizeof(int32_t)*9 for display matrix)
- Fix null pointer dereference of chosen_codec_data in PASSTHROUGH mode
(use video_in_ctx->pix_fmt instead)
zm_stream.cpp:
- Fix logic error in last_crop initialization check (OR should be AND)
FFmpeg is an integral component of ZM. Promote the appropriate libraries to required dependencies.
This reduces the possible build configurations greatly and thus maintenance burden.
