Skip to content

msg: synchronize turbo snapshot readers and complete $! access paths - #7514

Open
jjourdin wants to merge 4 commits into
rsyslog:mainfrom
jjourdin:pr/mmnormalize-turbo-msgdup-simplify
Open

msg: synchronize turbo snapshot readers and complete $! access paths#7514
jjourdin wants to merge 4 commits into
rsyslog:mainfrom
jjourdin:pr/mmnormalize-turbo-msgdup-simplify

Conversation

@jjourdin

@jjourdin jjourdin commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary (non-technical, complete)

Turbo snapshot duplication must stay correct when two workers share one
message, without paying a JSON build on every in-memory copy. The lock
added on the previous snapshot-sharing change is sound, but mixing a
materialize-on-allocation-failure fallback into that path made the
ownership model harder to reason about than it needs to be.

References

Refs: #7482

Notes (optional)

In-memory MsgDup still shares the snapshot. A failed counter allocation
now fails the duplicate instead of building JSON.

Before: MsgDup published the lazy refcount under the source lock and, if
that malloc failed, materialized so the copy could keep fields through a
JSON deep copy.

After: MsgDup still locks to publish the counter and copy JSON.
Materialize is an idempotent one-way projection into pMsg->json and no
longer clears turbo callbacks.

Test: mmnormalize-turbo-msgdup-share.sh (async $! materialize via
MsgAddRef plus a call into a queued ruleset). ASan/LSan and TSan clean
on that test and on mmnormalize-turbo-message-lifecycle.sh.

Commit message has Why / Impact / Before / After / Technical Overview.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread tests/mmnormalize-turbo-msgdup-share.sh
@jjourdin

Copy link
Copy Markdown
Contributor Author

@rgerhards : This is a proposal for the follow-up you asked for on #7482, around d67fd2c.

I kept the source lock: two workers can still share one message (async action with the default copyMsg=off, then a call into a queued ruleset), so first-counter publication and the JSON copy need a coherent view.

What I dropped is the materialize-on-allocation-failure fallback in MsgDup. A failed counter malloc now fails the duplicate, same as any other MsgDup OOM. Materialize stays a one-way projection into pMsg->json and no longer clears the turbo callbacks.

The never-duped path is unchanged: no extra malloc, no atomic, no lock.

mmnormalize-turbo-msgdup-share.sh covers that shared-message topology.

Happy to change the shape if you would rather keep the fallback or simplify the lock further.

@rgerhards rgerhards self-assigned this Aug 19, 2026
@rgerhards rgerhards added this to the v8.2610 milestone Aug 20, 2026

@rgerhards rgerhards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for simplifying the materialization state. The locked MsgDup() path is coherent, but this does not yet close the concurrency and subtree correctness issues, so I am requesting changes.

Blocking issues:

  1. The lock-free turbo getter can race snapshot overwrite/release. getJSONPropVal() and msgGetJSONPropJSONorString() read pMsg->json, turbo_result, and the callback without acquiring pMsg->mut or pinning the snapshot generation. A queued action with the default copyMsg=off shares the same smsg_t through MsgAddRef(); a later turbo mmnormalize on the continuing ruleset can take the writer lock, call MsgReleaseTurboResult(), free the old sole-owner snapshot, clear the slots, and attach another snapshot while the queued action is using the old pointer. Writer-only locking does not synchronize the unlocked reader, and pointer-sized plain loads are not sufficient C synchronization. Please either lock the read, or introduce a coherent immutable descriptor with safe lifetime pinning/retirement.

  2. Subtree JSON access remains unsafe and does not materialize turbo state. tplToJSON() still calls jsonFind(), receives a borrowed pointer after jsonFind() unlocks, and then calls json_object_get(). A concurrent mutation can replace/free the subtree in that interval, and even a retained json-c reference would not make subsequent in-place mutations into a snapshot. Also, jsonFind() never calls msgMaterializeTurboJSON(), so template(type="subtree" subtree="$!") can see an empty tree for a turbo-only message. Please route this path through an owned deep copy such as msgGetJSONPropJSON() while holding the mutex, with ownership adjusted so no extra json_object_get() leaks the copy.

  3. The new regression test does not cover the claims strongly enough.

    • It performs one MsgDup() per source message; the other branch uses MsgAddRef(), so it cannot exercise two duplicators racing first turbo_result_refs publication.
    • template(name="tree" type="string" string="%$!%\n") takes the string/materialization path, not the unsafe type="subtree" / tplToJSON() path.
    • The full-tree oracle checks the line count but uses grep -q '"num"', so only one of 5,000 lines needs the field. Thousands of {} lines would still pass.
    • There is no second turbo mmnormalize racing a lock-free getter on the same shared message.

Please add deterministic ASan/TSan coverage for:

  • two same-message duplicators,
  • type="subtree" subtree="$!",
  • a shared-message lock-free getter racing a second turbo normalization/overwrite,
  • and an oracle that validates every output record/sequence.

The new turbo_json_ready bookkeeping is useful, but retaining MsgLock() across both jsonDeepCopy() calls means this PR also does not address normal-case copy contention. That performance redesign can be separate, but the lifetime and subtree correctness gaps above should be resolved before merging.

@rgerhards rgerhards added the bug label Aug 20, 2026
@jjourdin
jjourdin marked this pull request as draft August 20, 2026 13:08
@jjourdin
jjourdin force-pushed the pr/mmnormalize-turbo-msgdup-simplify branch from ef35643 to 4b1a650 Compare August 21, 2026 09:47
@jjourdin jjourdin changed the title msg: simplify turbo snapshot sharing in MsgDup msg: synchronize turbo snapshot readers and complete $! access paths Aug 21, 2026
@jjourdin

Copy link
Copy Markdown
Contributor Author

@rgerhards thanks for the detailed review. The branch now closes all three points. The tests were written first: the ones that target the reported defects (exists, jsonmesg, unset, shared-getter-tsan, shared-set-tsan, subtree-template under TSan, segdisk-lifecycle, and the unit checks for jsonFind/exists/jsonmesg/unset/shared readers) are red on ef35643 and green after the change; msgdup-share, second-normalize, msgdup-concurrent-tsan, message-lifecycle and hup-tsan are regression coverage and pass on both. The two tplToJSON unit checks were written after the fix; the per-field over-reference they guard is argued from the code (msgGetJSONPropJSON returns a deep copy, json_object_object_add takes one reference) and the post-fix run is LSan-clean.

1. Lock-free getter vs snapshot overwrite/release. The fast path in getJSONPropVal() and msgGetJSONPropJSONorString() is now msgTurboGetStr(). It reads the json == NULL guard, the snapshot slots and the get_str result (including the copy out of the snapshot) under pMsg->mut whenever the message can be reached by another thread, i.e. iRefCount > 1. A sole holder keeps the lock-free read: a second reference can only be published by a thread that already holds one, so iRefCount == 1 observed by the caller proves exclusive access (the same invariant msgDestruct relies on when it tears down mut unlocked). Without HAVE_ATOMIC_BUILTINS it always locks. The "lock-free by design" comment is gone; the locking contract is documented once at the top of msg.c.

Reproduced before the fix (reports kept): runtime_unit_turbo_msgdup shared_readers_vs_overwrite is an ASan heap-use-after-free in the getter (fake_get_str <- getJSONPropVal msg.c:3223, freed by MsgReleaseTurboResult on the owner thread) and a TSan race; mmnormalize-turbo-shared-getter-tsan.sh (async omfile copyMsg=off + second turbo mmnormalize) is an ASan SEGV on the action worker with the stack strcmp <- ln_fast_result_get_string <- turbo_result_get_str_cb <- getJSONPropVal msg.c:3223 <- MsgGetProp <- tplToString <- prepareDoActionParams, and a TSan race. shared_readers_vs_materialize and mmnormalize-turbo-shared-set-tsan.sh cover the json == NULL guard against a concurrent materialize / set (TSan race before, clean after). The stock mmnormalize-turbo-subtree-template.sh topology (async $! template + later field read, no second normalize) was also a TSan race before.

2. Subtree / jsonFind. tplToJSON() takes an owned deep copy through msgGetJSONPropJSON() (which materializes under the mutex), and the per-field path hands the deep copy to the object without the extra json_object_get() (that was one leaked reference per field per message on JSON-passing outputs). jsonFind() materializes for $!, so exists() and msgCheckVarExists() see turbo fields. While going through the $! access paths I found three more that ignored the snapshot and fixed them in the same way: %jsonmesg% emitted "$!": null for a turbo-only message (now materializes and serializes $! under the mutex), unset $! was a no-op on a turbo-only message and left the removed fields readable through the snapshot after a materialized tree was dropped (now releases the snapshot), and unset $!field was a no-op on a turbo-only message (now materializes first). Materialize is guarded on $! everywhere, including msgAddJSON (a $/ write runs under the global-variables mutex, not the message mutex, so it must not touch the snapshot), and exists() answers from the snapshot when it serves the field (a probe, no copy) so it does not push the message onto the JSON path. A materialize allocation failure is now reported once through LogError and propagated by msgAddJSON, msgDelJSON and the queue codecs; the getters still render the field as absent, as before. Two more turbo-specific holes closed while at it: the segmented disk queue codec persisted msg->json as is, so a turbo-only message spilled to queue.type="segmentedDisk" came back with an empty $! (segdiskCodecEncode now calls the new MsgTurboMaterialize(), same contract as MsgSerialize; mmnormalize-turbo-segdisk-lifecycle.sh fails on ef35643 and passes now); and MsgDup copied APPNAME/PROCID/MSGID before taking the source mutex while their getters create them lazily under it on a shared message (TSan report kept: MsgDup msg.c:1079 read vs cstrConstruct <- MsgSetAPPNAME <- prepareAPPNAME <- getAPPNAME <- msgGetJSONMESG write under the mutex); the three copies now happen under the lock.

Note on coverage: string-passing outputs (omfile, omfwd) render type="subtree" through tplToString(), so only JSON-passing outputs (ommongodb, omjournal) reach tplToJSON(). The unit test therefore drives tplToJSON() (subtree and per-field), jsonFind(), msgGetJSONPropJSON() and msgGetJSONMESG() directly; mmnormalize-turbo-subtree-template.sh covers the omfile side.

3. Tests.

  • tests/unit/turbo_msgdup_test.c (runtime_unit_turbo_msgdup, built when LOGNORM_TURBO_SUPPORTED): a fake snapshot behind the same slots and callbacks mmnormalize uses, no liblognorm needed. 14 single-threaded checks of the $! contract (getters, jsonFind, exists, roots, jsonmesg, the three unset cases, merge semantics of a second attach, MsgDup sharing, and tplToJSON itself with a subtree template and a per-field list template on a turbo-only message and on a plain JSON tree: owned deep copy, LSan clean), plus two threaded checks: MsgAddRef readers on every getter racing the owner's snapshot replacement (with MsgDup workers), and readers racing materialization. Deterministic under ASan/TSan in seconds. It links the runtime and grammar the way rsyslogd does and stubs the rsyslogd.c globals; if you would rather not carry a runtime-linked unit test, it is self-contained and can be dropped without touching the rest.
  • Shell tests, each with a per-record oracle (seq_check on the extracted field, or every line matched): mmnormalize-turbo-exists.sh, -jsonmesg.sh, -unset.sh (turbo-only and materialize-then-unset), -second-normalize.sh (materialize between two normalizations, exact merged key set), -subtree-template.sh, -shared-getter-tsan.sh, -shared-set-tsan.sh, -msgdup-concurrent-tsan.sh (two omruleset action workers with copyMsg=off each MsgDup() the same smsg_t, plus a call), -segdisk-lifecycle.sh. The existing -msgdup-share.sh now checks every full-tree record and extracts the sequence instead of grep -q. The two-duplicator test is green on the previous code too: MsgDup already held the source mutex around first-counter publication, so it is regression coverage, not a red test.

Results on Linux/amd64, clang 21, -fsanitize=address and -fsanitize=thread builds, halt_on_error=1: all 16 unit checks and the 12 turbo shell tests (including mmnormalize-turbo-hup-tsan.sh and -message-lifecycle.sh) pass; before the change 6 unit checks and 3 shell tests fail functionally and 2 unit checks + 3 shell tests report ASan/TSan errors. A broader subset of the existing testbench around $! access (json, rscript exists/set/unset, templates, mmjsonparse, mmnormalize, omruleset, msgvar-concurrency: 75 scripts) gives 67 pass, 4 skip and 4 failures on the turbo ASan build; the 4 failures (mmpstrucdata not built in that configure, liblognorm regex support) fail the same way on ef35643.

Performance: getter microbenchmark on the plain build (clang 21 -O2, amd64, 5M calls of getJSONPropVal on a turbo-only message, fake snapshot): sole holder (iRefCount == 1) 21.5 ns/call before vs 21.1 after (the gate is one acquire load and a branch, no lock; earlier runs put it within 19-24 ns on both builds, noise level on that 2-vCPU box); shared message, uncontended (refcount 2, one thread) 19.3 ns before vs 27.4 after (the mutex); shared and contended (two readers on one message) 37.6 ns before vs 176.5 after, which is the price of not reading freed memory. For reference the materialized json-c path costs 68-75 ns/call. End to end, turbo mmnormalize + omfile with a five-field template, 200k messages, 3 runs each: direct queue 79-113k msg/s before vs 123-128k after, async action queue (shared message) 70-74k before vs 74-92k after (3 runs each; the spread is run-to-run noise on that 2-vCPU box), no regression.

Three things I noticed and left out because they are pre-existing and unrelated to this PR's turbo scope; happy to send them separately: the oversize-message JSON report (errmsg.c) borrows the live $! tree after unlocking (same borrowed-pointer class), but it runs at message reception, before any ruleset, so a turbo snapshot is never present there — a legacy-only fix; MsgSerialize uses the json-c print buffer after MsgUnlock; and exists() leaks the property name at config load (LSan with detect_leaks=1).

Things worth saying out loud rather than leaving for you to find:

  • type="subtree" on a JSON-passing output now costs one deep copy of the subtree per message (msgGetJSONPropJSON, as you asked), where it used to be a refcount bump on the live tree. Measured on the same box: a 3-field $! tree costs ~315 ns per msgGetJSONPropJSON copy (the former path was a refcount increment); it scales with the tree size. ommongodb/omjournal users with large subtrees will see it; keeping a borrowed reference would mean serializing under the message mutex in the output, which is a larger change and better done separately if the cost matters.
  • %jsonmesg% now renders under the message mutex (materialize, the $! add, the serialization), so a concurrent reader of the same shared message waits for it; before, the serialization wrote json-c print buffers of the live tree without the lock.
  • All the new tests, like the existing turbo ones, are gated on LOGNORM_TURBO_SUPPORTED; the CI jobs install the distro liblognorm-dev, which has no turbo, and the macOS job configures --disable-mmnormalize, so none of them run in the PR pipeline today. They run locally against a turbo-enabled liblognorm. The OS_APPLE branch of the unit test link (-force_load of the runtime archive, the ld64 spelling of --whole-archive) is therefore untested, and the non-Apple branch assumes GNU ld (--whole-archive, -export-dynamic); an AIX or Solaris build with a turbo liblognorm would need its own spelling, as tools/Makefile.am does for rsyslogd. If you would rather not carry that, the unit test is self-contained and can be dropped.
  • exists() on a field the snapshot does not serve, type="subtree" on a JSON-passing output, %jsonmesg% and unset $!field materialize the tree, which takes that message off the snapshot fast path for the rest of its life (later-writer-wins, as before for any $! write). exists() on a served field and the string getters do not.
  • The unconditional changes (template.c ownership, the getters' materialize guards) were also exercised on a --disable-turbo build: configure --disable-turbo (LOGNORM_TURBO_SUPPORTED false, HAVE_LOGNORM_TURBO undefined), the same 75-script subset around $! access gives the same 67 pass / 4 skip / 4 environment failures as the turbo build and as ef35643, and the unit test has a plain-JSON variant of the tplToJSON checks (tpltojson_plain_json, no snapshot involved).

The jsonDeepCopy contention in MsgDup is untouched, as agreed.

@jjourdin
jjourdin marked this pull request as ready for review August 21, 2026 09:50
Why:
The turbo field getters read the snapshot slots and pMsg->json without the
message mutex. A message shared by reference with an action queue
(copyMsg=off, the default) can be renormalized by a second turbo
mmnormalize action on the continuing ruleset: the release of the first
snapshot races the action worker inside the getter (use after free, seen
as a SEGV in ln_fast_result_get_string under ASan). Several $! access paths
also never looked at the snapshot: jsonFind (so exists() was false and
type="subtree" templates empty on turbo-only messages), %jsonmesg%
("$!": null), and unset $! / unset $!field (silent no-ops that left the
removed fields readable through the snapshot).

Impact:
Turbo-normalized messages are now safe to share by reference and every $!
access path sees the normalized fields. The never-shared path keeps its
lock-free field read; shared messages take the mutex on reads.

Before:
- getJSONPropVal / msgGetJSONPropJSONorString read turbo_result,
  turbo_result_get_str and pMsg->json with plain loads.
- jsonFind, msgGetJSONMESG and msgDelJSON ignored the snapshot.
- tplToJSON subtree borrowed the live tree after jsonFind unlocked and
  took an extra reference on the deep copy returned for each field.

After:
- msgTurboGetStr serves the field under pMsg->mut whenever iRefCount > 1
  (a second reference can only be published by a thread that holds one,
  so iRefCount == 1 proves exclusive access and stays lock-free); the copy
  is taken inside the locked region.
- jsonFind, msgGetJSONPropJSON and msgGetJSONMESG materialize the
  snapshot under the mutex; msgGetJSONMESG serializes $! under the mutex.
- msgDelJSON releases the snapshot on unset $! and materializes before a
  leaf unset. Materialize runs only for $! accesses (msgAddJSON included:
  a $/ write holds the global-variables mutex, not the message mutex).
- msgCheckVarExists answers from the snapshot when it serves the field, so
  exists() does not force the message onto the JSON path.
- The segmented disk queue codec materializes the snapshot before it
  persists the JSON tree (MsgTurboMaterialize), like the classic queue.
- MsgDup copies APPNAME, PROCID and MSGID under the source mutex: their
  getters create them lazily under that mutex on a shared message.
- tplToJSON subtree takes an owned deep copy through msgGetJSONPropJSON;
  the per-field path hands the deep copy to the object without an extra
  reference (one reference per field per message was leaked).
- runtime_unit_turbo_msgdup exercises every path on a fake snapshot,
  single-threaded and with MsgAddRef readers racing snapshot replacement
  and materialization; it fails on the previous code under ASan (heap use
  after free in the getter) and TSan, and also drives tplToJSON directly.
  New testbench scripts cover exists(), %jsonmesg%, unset, second normalization merge, subtree templates, the
  shared getter against a second normalization, set on a shared message
  (including a $/ write), two concurrent MsgDup callers and a segmented
  disk queue, each with a per-record oracle.

Technical Overview:
runtime/msg.c: msgTurboGetStr (refcount-gated lock), materialize guarded on
PROP_CEE in the getters, jsonFind/msgGetJSONMESG/msgDelJSON turbo-aware,
MsgTurboMaterialize, MsgDup copy order, locking comment rewritten.
runtime/segdisk_codec.c: materialize before encoding the JSON tree.
runtime/template.c: tplToJSON subtree and field ownership.
tests/unit/turbo_msgdup_test.c, tests/mmnormalize-turbo-*.sh, Makefile.am.
@jjourdin
jjourdin force-pushed the pr/mmnormalize-turbo-msgdup-simplify branch from 4b1a650 to 9507ee2 Compare August 26, 2026 19:36

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 17 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread runtime/msg.h Outdated
Comment thread tests/Makefile.am
…t source

The doc said the projection happens "under the message mutex", which reads
either way, and it sits directly below MsgReleaseTurboResult, whose doc tells
the caller to hold the mutex. A caller following the neighbour would lock
first and then block on a mutex that is created with default attributes and is
therefore not recursive. Both call sites today enter without the lock, so this
is a trap for the next caller rather than a live bug. Say plainly that the
function takes and releases the lock itself.

unit/turbo_msgdup_test.c reaches the build only through a check_PROGRAMS
inside a conditional, so like the other conditional unit sources it needs an
EXTRA_DIST entry to appear in a dist tarball.
@rgerhards

Copy link
Copy Markdown
Member

@jjourdin Solaris is a VM issue by it's provider. looks longer lasting, I'll check that I disable. The kafka instance is also a CI issue, I'll investigage. Both are not merge blockers.

@rgerhards rgerhards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if possible, it would be good to get around the codecov issues, so that they do not persist. But this is NOT a merge blocker.

Jérémie Jourdin added 2 commits August 31, 2026 15:54
The iRefCount == 1 lock-free path in msgTurboGetStr() still raced a
locked overwrite from a second turbo mmnormalize on a message shared by
MsgAddRef (copyMsg=off). An acquire load of the refcount is not a
happens-before with that write. ubuntu_26_tsan failed
mmnormalize-turbo-shared-getter-tsan.sh: write of turbo_result under
pMsg->mut, previous read without the mutex. ASan can hit the same window
as a use-after-free in ln_fast_result_get_string.

Always take pMsg->mut for the json == NULL guard, the slot loads, the
get_str callback, and the copy out of the snapshot. The mutex is not
recursive; today's callers enter unlocked.

Signed-off-by: Jérémie Jourdin <jeremie.jourdin@advens.fr>
@rgerhards

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T10:53:21.274253Z 8987c99 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8987c992c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread doc/ai/module_map.yaml
- MsgGetRcvFromProp returns fromhost, fromhost-ip, or fromhost-port with the stored prop length; it resolves DNS only when NEEDS_DNSRESOL is still set.
- MsgDup locks the source message while coherently copying JSON state and the lazy turbo snapshot callbacks.
- Turbo parse snapshots are immutable and may be shared by MsgDup copies through an atomic reference count; each owner must release only through MsgReleaseTurboResult while holding the message lock or exclusive access.
- Turbo snapshots are write-once at attach and immutable afterwards. Attach, overwrite, release and every read from a shared message (iRefCount > 1) happen under the message mutex; a sole holder reads the snapshot without the lock (msgTurboGetStr). Materialize is an idempotent one-way projection into pMsg->json under the message mutex and does not clear turbo callbacks; jsonFind, msgGetJSONMESG and msgDelJSON materialize too, and unset $! releases the snapshot.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document that every turbo getter takes the mutex

This entry says a sole holder reads the snapshot without locking, but the committed msgTurboGetStr() now unconditionally calls MsgLock(pMsg). Because this map is the locking contract intended for module authors, retaining the obsolete optimization can lead consumers to make incorrect concurrency and performance assumptions; update it to match the always-locked implementation.

AGENTS.md reference: runtime/AGENTS.md:L48-L49

Useful? React with 👍 / 👎.

Comment on lines +40 to +42
for f in "$RSYSLOG_DYNNAME.spool"/turbo-segdisk*; do
[ -e "$f" ] && segfiles=$((segfiles + 1))
done

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for segmented queue materialization before inspecting files

On a loaded runner, successful completion of tcpflood only proves that the client finished sending; rsyslog may not yet have processed a message through the asynchronous segmented-disk action. This loop can therefore observe no matching path and fail even though the queue is functioning correctly. Wait for the queue directory or another queue-specific readiness condition before asserting its existence.

AGENTS.md reference: tests/AGENTS.md:L101-L104

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants