Make Pipe serialization-completion flags volatile to fix cross-lock visibility race

[gynoro]

Purpose

serializationComplete and rawSerializationComplete in Pipe are written under synchronized(this) (or with no lock, in setRawSerializationComplete) but read in consume() / consumePostActions() while the calling thread holds the ReentrantLock lock. Because the fields are not volatile and the write/read paths use different monitors, the JMM provides no happens-before edge between them. On the content-altering out path (outputBuffer != null) the consumer can observe a stale completion flag at the empty-buffer boundary and either complete the response early (truncation) or fail to suspend the encoder (short/spliced body), corrupting responses at ~8 KB IO-buffer multiples under load. The pure-relay path (outputBuffer == null) is unaffected because it gates on producerCompleted, which is published under the same lock.

Resolves wso2/api-manager#5099

Goals

Close the cross-lock memory-visibility gap on the two serialization-completion flags so the consumer always observes the latest value regardless of which lock the writer held.

Approach

Mark both serializationComplete and rawSerializationComplete as volatile. This establishes a happens-before edge for every read and write independent of the monitor held, with no change to locking strategy or control flow. Minimal, behavior-preserving fix.

Release note

Fixed a rare data race on the PassThrough Pipe serialization-completion flags that could produce truncated or spliced HTTP responses at IO-buffer boundaries under load.

Documentation

N/A — internal concurrency fix, no user-facing or API change.

Training

N/A

Certification

N/A — no behavioral or API change.

Automation tests

• Unit tests: N/A — the defect is a timing-dependent memory-visibility race that is not deterministically reproducible in a unit test. The change is a single-keyword, behavior-preserving fix.
• Integration tests: Existing PassThrough transport integration tests continue to apply.

Security checks

• Followed secure coding standards in http://wso2.com/technical-reports/wso2-secure-engineering-guidelines? yes
• Ran FindSecurityBugs plugin and verified report? no
• Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets? yes

Samples

N/A

Related PRs

N/A

Migrations (if applicable)

N/A

Test environment

Field-validated against WSO2 API Manager 2.6.0 (synapse-core 2.1.7-wso2v80) on JDK 8 / Linux. The corruption (~5–10 per million responses, at ~8 KB multiples) disappears when a no-op <enrich> mediator forces an in-memory copy, consistent with the diagnosed race on the content-altering path.

Learning

Diagnosed from production field evidence by correlating truncated/spliced response sizes with IO-buffer multiples, then tracing the lock asymmetry between the flag writers (synchronized(this)) and readers (ReentrantLock). See wso2/api-manager#5099 for the full analysis.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec67ca69-7baa-4703-897e-8879f760e12d

📥 Commits

Reviewing files that changed from the base of the PR and between 67ee70a and 79f4870.

📒 Files selected for processing (1)

• modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/Pipe.java

---

📝 Walkthrough

Summary

This PR fixes a memory visibility race condition in the Pipe class of the PassThrough transport by marking two serialization-completion state flags as volatile.

Changes Made

• Modified serializationComplete and rawSerializationComplete boolean fields to volatile in modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/Pipe.java

Why This Matters

These fields were previously written under synchronized(this) but read by consumer threads holding a different lock (ReentrantLock). This cross-lock visibility mismatch meant the Java Memory Model provided no happens-before guarantees, allowing consumer threads to observe stale flag values when buffers drained to empty. This could result in either premature response completion or failure to properly manage message transmission on the content-altering output path.

Impact

The fix is minimal and behavior-preserving. By making these fields volatile, every read and write operation now establishes proper happens-before edges regardless of which monitor is held. This ensures consumers always observe the latest serialization state, eliminating the race condition that was causing message corruption at buffer boundaries during high-concurrency scenarios with content-altering mediators.

Walkthrough

In Pipe.java, the two serialization-completion state flags—serializationComplete and rawSerializationComplete—are changed from plain boolean fields to volatile boolean fields. This establishes a Java Memory Model happens-before relationship between the threads that write these flags (using synchronized(this) or no synchronization) and the consumer thread that reads them under a ReentrantLock, which previously used two distinct lock monitors with no shared visibility guarantee.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: making serialization-completion flags volatile to fix a cross-lock visibility race condition in the Pipe class.
Description check	✅ Passed	The pull request description comprehensively addresses all required template sections with technical depth, including purpose, goals, approach, release notes, documentation justification, security checks, test environment, and learning context.
Linked Issues check	✅ Passed	The code changes directly address the cross-lock visibility race documented in #5099 by making both serializationComplete and rawSerializationComplete volatile, establishing happens-before edges independent of monitor ownership.
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped to the identified defect: only the two boolean fields are modified from non-volatile to volatile, with no alterations to locking strategy, control flow, or unrelated code.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}