Repository navigation
fix: free the MonitorLink each linked-monitor token creates - #5219
Merged
connortechnology merged 2 commits intoOct 12, 2026
Merged
Conversation
Token's constructor allocates a Monitor::MonitorLink with new for every monitor in a monitor's LinkedMonitors expression and keeps it in a raw pointer that nothing deletes (~Token is defaulted, and parse() copies tokens into the tree, so the pointer is shared). Monitor::connect() re-parses the expression through ReloadLinkedMonitors() on every connect, so each reconnect or SIGHUP reload of zmc leaks one MonitorLink per linked monitor, with the Monitor it holds, and once the link has been used, the open file descriptor and the mapping of the linked monitor's shared memory. Hold it in a std::shared_ptr instead, so the copies share it and the last one frees it when the expression tree is deleted (disconnect() and ReloadLinkedMonitors() delete it). The defaulted copy and move constructors can no longer be constexpr. fixes ZoneMinder#5218 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Token::hasAlarmed(), Token::score() and MonitorLinkExpression::Node(Token const &) were constexpr; with monitor_link_ a std::shared_ptr they call non-constexpr functions, which the -Werror builds reject (-Winvalid-constexpr, gcc and clang). None of them is used in a constant expression. refs ZoneMinder#5218 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Tokenholds itsMonitorLinkin astd::shared_ptr, so the token copies share it and the last one frees it with the expression tree; 1 file, +6/-5Details: reproduction, change, checks
Before
Upstream master
c3d2e542b, clean plain build; monitor 2 linked to monitor 1, ten SIGHUPreloads of
zmc -m 2(zmdc.pl reload), 8 s apart:Change
src/zm_monitorlink_token.h:monitor_link_becomesstd::shared_ptr<Monitor::MonitorLink>,made with
std::make_shared. Tokens are copied (parse()copies them into the tree), which iswhy a
deletein~Tokenalone would double-free; a shared owner fits that. The defaultedcopy and move constructors lose
constexpr(not allowed with ashared_ptrmember in C++17).(
Monitor::Load), which the unit tests don't have. Covered by the probe above and the eventscheck's linked scenario.
Behaviour changes
deletein~Token(double free: tokens are copied);unique_ptr(needs the tree to move tokens instead of copying them, a larger change to the parser)shared_ptrlinkedPASSlinkedPASSSide fixes in this PR: none.
After
Branch
fix/b77-monitorlink-token-leakat91526d2e9(basec3d2e542b), tested in a merge withtwo other fixes from the same lab session (
combo= master + this +fix/b55-usr-level-signal-safefix/b65-zmc-stop-during-startup; no shared files with this one):check-changed.py: unit-tests, script-tests, smoke, resilience, events, dblog --quick, signals --quick)zmonvif-probe.pldiscovery, known), resilience PASS=5, events PASS=10 (record, mocord, modect-only, nodect, monitor, linked, jpegs, reload, filters, purge), dblog --quick PASS=1 XFAIL=2 (thread-error, contention: known on master, #5197/#5208), signals --quick PASS=4 (hup-default, usr, usr-threads, startup-stop)Not run: the deep set. Backport to release-1.38: same code, the commit cherry-picks cleanly (not built).
Tested as
7cde4e059;91526d2e9is the same tree withfixes #5218added to the message.fixes #5218
🤖 Generated with Claude Code