Skip to content

fix: unlink the zms command socket on exit - #5034

Merged
connortechnology merged 2 commits into
ZoneMinder:masterfrom
connortechnology:fix-zms-socket-unlink
Aug 3, 2026
Merged

fix: unlink the zms command socket on exit#5034
connortechnology merged 2 commits into
ZoneMinder:masterfrom
connortechnology:fix-zms-socket-unlink

Conversation

@connortechnology

Copy link
Copy Markdown
Member

Split out of the investigation in #5029. Independent of #5033 and of the mode=single question in #5029.

closeComms() left $PATH_SOCKS/zms-NNNNNNs.sock behind on exit. The only thing that ever removed it was the unlink() before bind() in a later zms that happened to draw the same connkey, so these files accumulate indefinitely.

That matters because web/ajax/stream.php uses file_exists() on exactly that path to decide whether zms is listening, and waits for it to appear before giving up:

while ( !file_exists($remSockFile) && $max_socket_tries-- ) {
  usleep(1000);
}

A file left by an exited zms defeats that wait: the check passes immediately, the subsequent socket_sendto() gets ECONNREFUSED, and the command is reported as failed even though a new zms was about to bind. genConnKey() draws from six digits, and a page cycling monitors every five seconds burns ~720 connkeys an hour, so a reuse is likely well within an hour of viewing.

This is the mechanism behind the "Socket ... does not exist" / "Timed out waiting for msg" errors in #5029, which in turn trigger the connkey regeneration that orphans the running zms.

Why unlinking is safe here. The existing comment says we can't delete the files because another zms may have opened them and be waiting on the lock. That holds for the .lock file, which this PR still leaves alone. It doesn't hold for the command socket: a second zms with the same connkey blocks on flock(LOCK_EX) in openComms() before it unlinks and binds, so while we still hold the lock it cannot yet have created its own socket, and we cannot be deleting a file that belongs to it.

Limitation: best effort. A zms killed by a signal still leaves the socket behind. In that case the process is usually still running, so the file being present isn't wrong.

Also resets lock_fd to -1 after closing it.

Testing: builds clean (cmake --build . --target zms, Debug). Not verified against a live install — the useful check is watching $PATH_SOCKS stop accumulating *s.sock files over a long montage session.

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>
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>
@connortechnology

Copy link
Copy Markdown
Member Author

Raised in review: we deliberately don't delete these files, because a second zms may be waiting on the lock and would then be unable to access it.

Worth separating the three files involved, because the rule is guarding a different one than this patch touches:

File Created/bound by Role
zms-NNNNNN.lock zms — open() + flock(LOCK_EX) serialises two zms sharing a connkey
zms-NNNNNNs.sock zms — bind() (loc_sock_path) PHP sends commands to this
zms-NNNNNNw.sock PHP — socket_bind() zms replies here; PHP's ajaxCleanup() unlinks it

The waiting-on-the-lock concern is about file 1. This patch removes file 2 and leaves file 1 alone. A second zms blocked in flock() holds an open fd on zms-NNNNNN.lock; unlinking a differently-named socket can't affect that wait.

The part that settles it is what the waiter does once it wins the lock, in openComms():

// Unlink before bind, in case it already exists
unlink(loc_sock_path);

It deletes its predecessor's socket file itself. It never reads it or connects to it. So removing it in closeComms() isn't taking anything away from the waiter — it's doing early exactly what the waiter does the moment it wakes. The only consumer that treats that path as meaningful is stream.php's file_exists() check, which is the bug being fixed.

Two supporting mechanics:

  • flock() locks the open file description, not the path. Unlinking wouldn't break a waiter that already has the file open — which is exactly why the existing code says "close it rather than unlock it in case it got deleted."
  • A bound unix datagram socket keeps working for its holder after its path is unlinked; only fresh by-name lookups fail. zms is exiting regardless.

The concern did surface a real bug, though, pushed as e75a8b9.

openComms() logs an error but carries on to bind() when it can't open or flock() the lock file, so a zms can be serving without holding the lock. In that state the unconditional unlink could remove a socket belonging to the zms that does hold the lock, leaving it unreachable — the exact failure described, reached by a different route. The unlink is now guarded on lock_fd >= 0; leaking the file when we never held the lock is no worse than the behaviour before this patch.

Ordering is load-bearing for the same reason and was already correct: unlinking after releasing the lock could delete a socket the successor had just bound. The comment now spells all of this out.

Testing: zms builds clean; suite still 117 cases / 1788 assertions passing. Still not verified against a live install.

@connortechnology
connortechnology merged commit 237295b into ZoneMinder:master Aug 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant