From c93b8d7de5d57460872580d3467a8188a86aecf9 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 2 Aug 2026 09:34:05 -0400 Subject: [PATCH 1/2] fix: unlink the zms command socket on exit refs #5029 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) --- src/zm_stream.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/zm_stream.cpp b/src/zm_stream.cpp index 5c0905b4d3e..115f6299a8e 100644 --- a/src/zm_stream.cpp +++ b/src/zm_stream.cpp @@ -413,9 +413,22 @@ void StreamBase::closeComms() { close(sd); sd = -1; } - // Can't delete any files because another zms might have come along and opened them and is waiting on the lock. + // Remove our command socket while we still hold the lock. Another 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 + // cannot be deleting a file that belongs to it. + // + // This has to happen: web/ajax/stream.php uses file_exists() on this path + // to decide whether zms is listening. A socket left behind by an exited zms + // makes that check succeed, so the sendto() fails with ECONNREFUSED instead + // of waiting for the new process to bind, and the command is lost. + if ( loc_sock_path[0] && (unlink(loc_sock_path) < 0) && (errno != ENOENT) ) { + Warning("Failed to unlink '%s': %s", loc_sock_path, strerror(errno)); + } + // Don't unlink sock_path_lock: another zms may already be waiting on it. if ( lock_fd >= 0 ) { close(lock_fd); //close it rather than unlock it in case it got deleted. + lock_fd = -1; } } } // end void StreamBase::closeComms From e75a8b965423e9706a66d60eac4fa2b248159809 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 3 Aug 2026 08:32:01 -0400 Subject: [PATCH 2/2] fix: only unlink the command socket while holding the lock 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) --- src/zm_stream.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/zm_stream.cpp b/src/zm_stream.cpp index 115f6299a8e..b15d05add9d 100644 --- a/src/zm_stream.cpp +++ b/src/zm_stream.cpp @@ -413,16 +413,29 @@ void StreamBase::closeComms() { close(sd); sd = -1; } - // Remove our command socket while we still hold the lock. Another 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 - // cannot be deleting a file that belongs to it. + // Remove our command socket, but only while we still hold the lock, and only + // if we actually got the lock. // - // This has to happen: web/ajax/stream.php uses file_exists() on this path - // to decide whether zms is listening. A socket left behind by an exited zms - // makes that check succeed, so the sendto() fails with ECONNREFUSED instead - // of waiting for the new process to bind, and the command is lost. - if ( loc_sock_path[0] && (unlink(loc_sock_path) < 0) && (errno != ENOENT) ) { + // Note this is zms-s.sock, not the .lock file. A second zms sharing + // this connkey is blocked in flock(LOCK_EX) on the .lock file, and the first + // thing it does after winning that lock is unlink this very path itself + // ("Unlink before bind" in openComms()). It never reads our socket file, so + // removing it here only does early what our successor would do anyway. + // + // Ordering matters: if we unlinked after releasing the lock we could delete + // a socket the successor had already bound, leaving it unreachable. + // + // The lock_fd guard matters because openComms() carries on and binds even + // when it fails to take the lock, so without it a lockless zms could delete + // a socket belonging to the zms that does hold the lock. Leaking the file in + // that case is no worse than the behaviour before this check existed. + // + // This is worth doing because web/ajax/stream.php uses file_exists() on this + // path to decide whether zms is listening. A socket left behind by an exited + // zms makes that check succeed, so the sendto() fails with ECONNREFUSED + // instead of waiting for the new process to bind, and the command is lost. + if ( (lock_fd >= 0) && loc_sock_path[0] + && (unlink(loc_sock_path) < 0) && (errno != ENOENT) ) { Warning("Failed to unlink '%s': %s", loc_sock_path, strerror(errno)); } // Don't unlink sock_path_lock: another zms may already be waiting on it.