Fix ByteBuffer concurrency issue in ConcurrentWebSocketSessionDecorator for binary messages

[xeromank]

For reference, I'm not good at English, so I wrote this message using a translator. Please understand if my expressions are a bit awkward or if there are any issues.

📝 Summary

This is an alternative approach to PR #35519, addressing the same thread-safety issue with a more fundamental solution.

With Spring's growing adoption of virtual threads (Project Loom), concurrency levels in applications have increased dramatically. This exposed a critical thread-safety issue when sharing BinaryMessage instances across multiple WebSocket sessions.

The Problem: When multiple threads send the same BinaryMessage to different WebSocket sessions, they share a single ByteBuffer whose position gets corrupted during concurrent reads, resulting in data corruption or empty messages.

Why This Is a Trap: Developers using binary messages might not be aware that ByteBuffer's position is mutable state. It's natural to assume that sending the same message to multiple sessions is safe - after all, we're just "reading" the data. This hidden implementation detail makes this bug easy to accidentally introduce and particularly challenging to diagnose.

I was also unaware of this implementation detail and only discovered it after spending two days debugging in production when concurrency levels increased dramatically. I hope this fix saves others from the same frustrating experience.

🔄 Changes

• Modified BinaryMessage.getPayload() to return ByteBuffer.duplicate() instead of the direct buffer reference
• Each caller now gets its own independent buffer position while sharing underlying data
• Ensures thread-safety at the source level without requiring defensive copying in every consumer

@Override
public ByteBuffer getPayload() {
    return this.payload.duplicate();
}

✅ Testing

Added test concurrentBinaryMessageSharingAcrossSessions() that reproduces the issue and validates the fix.

• Without fix: Test fails with corrupted buffers
• With fix: All messages delivered correctly

📊 Impact

• Performance: Minimal - only creates buffer wrapper, no data copying
• Compatibility: Fully backwards compatible
• Virtual Threads: Essential for applications using virtual threads with high concurrency

🏗️ Why Fix in BinaryMessage Instead of Decorators?

Previous Approach (PR #35519): The original PR fixed this in ConcurrentWebSocketSessionDecorator, but this approach had limitations:

• Only protected concurrent sends through the decorator
• Other components using BinaryMessage directly would still face the issue
• Required every consumer to implement defensive copying

Current Approach: Fixing at the source (BinaryMessage.getPayload()) provides:

• Universal Protection: Any code path using BinaryMessage is automatically protected
• Simpler Mental Model: Developers don't need to remember to use defensive copies
• Consistent Behavior: The message behaves like an immutable object from the consumer's perspective
• Framework-Level Solution: Addresses the issue once for all Spring WebSocket users

This follows the principle of "pit of success" - making the safe behavior the default behavior.

🎯 Why This Matters Now

As Spring applications increasingly adopt virtual threads:

• Concurrency levels can reach thousands of concurrent operations
• Traditional thread-safety assumptions may no longer hold
• Subtle race conditions become more prevalent

This fix ensures WebSocket handling remains robust in the virtual thread era by addressing the thread-safety issue at its root.

---

Note: This issue only manifests under high concurrency, particularly with virtual threads, making it challenging to detect in traditional testing scenarios. The fix at the BinaryMessage level ensures all usage patterns are protected, not just those using specific decorators.

[xeromank]

I'm uncertain which approach is the correct direction between the two PRs. Looking at the name ConcurrentWebSocketSessionDecorator, it seems like this class should be responsible for ensuring thread safety. However, I believe it would be fundamentally better if BinaryMessage usage itself was thread-safe, which is why I created this alternative PR. I'd appreciate the maintainers' thoughts on which approach aligns better with Spring's design philosophy.

#35519

[bclozel]

Closing for reasons explained in #35519 (comment)