fix(#5357): make EventMeshFrame immutable (defensive copy + unmodifiable view + withAttribute) - #5368
Merged
Merged
Conversation
…ble view) Phase 0 enforcement of the production-HA plan (#5354), sub-issue #5357: a frame that can be mutated after publish lets two concurrent dispatchers race on the same instance; the attribute map was also aliased from codec accumulators at construction. EventMeshFrame changes: - constructor deep-copies the attrs map (a caller-held codec accumulator can no longer alias frame state); null map/data are normalized - attributes() returns Collections.unmodifiableMap - new withAttribute(name, value) derives a new frame with one attribute set (null value removes); the source frame is unchanged Call-site migration (3 test files; production code had zero mutation sites - verified by grep over all 72 attributes() usages): - RestartCursorAlignmentTest / MqCursorRecordingTest: pop-cursor stamping now chains withAttribute - FrameProtocolConversionTest.framePopCkStampedForDeferredAck: converted from a mutability test to an immutability test (asserts withAttribute derives a stamped frame, source unchanged, and attributes() throws UnsupportedOperationException on put) ArchUnit guardrail (14 -> 15 rules): - ruleEventMeshFrameImmutable: only org.apache.eventmesh.common.wire.. may implement EventMeshFrame* types, so no other class can grow frame-mutating APIs Full local CI parity (Temurin 21.0.11): - :eventmesh-architecture-guard:test + :eventmesh-common:test + :eventmesh-runtime:test -> BUILD SUCCESSFUL - checkstyleMain/Test x3 modules (maxWarnings=0) -> BUILD SUCCESSFUL - clean build dist jacocoTestReport (CI task set) -> BUILD SUCCESSFUL (11m12s) Refs #5357 (sub-issue of #5354). Co-authored-by: qqeasonchen <qqeasonchen@gmail.com>
qqeasonchen
added a commit
that referenced
this pull request
Sep 8, 2026
…cess Phase 0 enforcement of the production-HA plan (#5354), sub-issue #5358: gauge-style quota resources (CONNECTIONS / SUBSCRIPTIONS / BACKLOG) leak when acquire has no guaranteed release. Production callers today (UniHttpServer x2, A2AGatewayHttpHandler, ConnectorScheduler) all use one-shot THROUGHPUT check() and never release - correct for windows, but nothing structural stops the next gauge-style call site from leaking the same way. New API (security.gate): - QuotaHandle implements AutoCloseable: close() releases exactly once, double-close is a no-op, non-releasable (THROUGHPUT) handles do not release (window counters self-expire) - SecurityGate.acquire(ctx, frame) -> QuotaHandle: runs the same auth/ACL-first check() then returns the paired handle; throws QuotaExceededException (new) when exhausted, consuming no slot - callers wrap gauge work in try-with-resources so the release runs on success, failure and cancellation alike ArchUnit guardrail (15 -> 16 rules): - ruleQuotaManagerOnlyFromGate: QuotaManager.tryAcquire/release may only be called from the security.gate package - any direct manager call outside the gate bypasses the pairing contract and fails the build Tests: - QuotaHandleTest (new, 4 cases): close-releases-exactly-once, try-with-resources releases on exception, THROUGHPUT handle does not release, exhausted acquire throws without consuming - checkstyleMain/Test + guard tests + full CI build all green Rebased onto develop after #5368 (rule ordering in ArchitectureRules.java); the 16-rule set now reads #5322(9) + #5348(2) + #5356(2) + #5357(1) + #5358(1). Full local CI parity (Temurin 21.0.11): - checkstyle + tests x (runtime, architecture-guard) -> BUILD SUCCESSFUL - clean build dist jacocoTestReport (CI task set) -> BUILD SUCCESSFUL (7m59s) Refs #5358 (sub-issue of #5354). Co-authored-by: qqeasonchen <qqeasonchen@gmail.com>
qqeasonchen
added a commit
that referenced
this pull request
Sep 8, 2026
…cess (#5369) Phase 0 enforcement of the production-HA plan (#5354), sub-issue #5358: gauge-style quota resources (CONNECTIONS / SUBSCRIPTIONS / BACKLOG) leak when acquire has no guaranteed release. Production callers today (UniHttpServer x2, A2AGatewayHttpHandler, ConnectorScheduler) all use one-shot THROUGHPUT check() and never release - correct for windows, but nothing structural stops the next gauge-style call site from leaking the same way. New API (security.gate): - QuotaHandle implements AutoCloseable: close() releases exactly once, double-close is a no-op, non-releasable (THROUGHPUT) handles do not release (window counters self-expire) - SecurityGate.acquire(ctx, frame) -> QuotaHandle: runs the same auth/ACL-first check() then returns the paired handle; throws QuotaExceededException (new) when exhausted, consuming no slot - callers wrap gauge work in try-with-resources so the release runs on success, failure and cancellation alike ArchUnit guardrail (15 -> 16 rules): - ruleQuotaManagerOnlyFromGate: QuotaManager.tryAcquire/release may only be called from the security.gate package - any direct manager call outside the gate bypasses the pairing contract and fails the build Tests: - QuotaHandleTest (new, 4 cases): close-releases-exactly-once, try-with-resources releases on exception, THROUGHPUT handle does not release, exhausted acquire throws without consuming - checkstyleMain/Test + guard tests + full CI build all green Rebased onto develop after #5368 (rule ordering in ArchitectureRules.java); the 16-rule set now reads #5322(9) + #5348(2) + #5356(2) + #5357(1) + #5358(1). Full local CI parity (Temurin 21.0.11): - checkstyle + tests x (runtime, architecture-guard) -> BUILD SUCCESSFUL - clean build dist jacocoTestReport (CI task set) -> BUILD SUCCESSFUL (7m59s) Refs #5358 (sub-issue of #5354).
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
1 participant
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.
What this PR does
Closes #5357 — Phase 0 enforcement of the production-HA plan (#5354):
EventMeshFramebecomes deeply immutable. A frame that can be mutated after publish lets two concurrent dispatchers race on the same instance; the attribute map was also aliased from codec accumulators at construction.1.
EventMeshFramechangesthis.attrs = attrs— aliases the caller's map (codec accumulators could mutate a live frame)new LinkedHashMap<>(attrs)); null map/data normalizedattributes()Collections.unmodifiableMap(attrs)withAttribute(name, value)— derives a new frame with one attribute set (null removes); source unchanged2. Call-site migration
Verified by grep over all 72
attributes()usages — production code had zero mutation sites; exactly 3 test files mutated the returned map:RestartCursorAlignmentTest/MqCursorRecordingTest: POP-cursor stamping now chainswithAttributeFrameProtocolConversionTest.framePopCkStampedForDeferredAck: converted from a mutability test to an immutability test (assertswithAttributederives a stamped frame, source unchanged,attributes().putthrowsUnsupportedOperationException)3. ArchUnit guardrail (14 → 15 rules)
ruleEventMeshFrameImmutable— onlyorg.apache.eventmesh.common.wire..may implementEventMeshFrame*types, so no other class can grow frame-mutating APIs.Verification (full local CI parity, Temurin 21.0.11)
Relations
docs/architecture-review/production-ha-plan.md(PR docs(#5354): production-HA acceptance plan — 10-issue topology (#5356-#5365) #5366)FrameLimitsbuilds on the immutable API)Co-authored-by: qqeasonchen qqeasonchen@gmail.com