fix(shm): Add memory barriers for cross-process shared memory visibility

[kitaekatt]

Summary

Adds proper memory barriers for cross-process shared memory synchronization to prevent data races.

Changes

• Add memory barriers in shared memory broadcast implementation
• Ensures correct visibility ordering across process boundaries

Testing

Tested with multi-process inference scenarios.

Note: PR recreated due to branch rename (was fix-shm-memory-barriers, now fix/29819-shm-memory-barriers)

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

[gemini-code-assist]

Code Review

This pull request correctly identifies and fixes a critical race condition in the shared memory broadcast mechanism. By introducing memory barriers using a memory_fence function, it ensures proper synchronization and visibility of memory writes between processes. The fences are strategically placed within the acquire_write and acquire_read methods, which is crucial for the correctness of the lock-free ring buffer on multi-core systems. The implementation of memory_fence using threading.Lock is a pragmatic approach to create a portable memory barrier. The changes are well-documented with clear comments explaining the purpose of each fence. Overall, this is a solid and necessary fix to prevent potential deadlocks and data corruption.

[kitaekatt]

This PR is a re-opening of #29819. The original branch was accidentally deleted, preventing that PR from being reopened.

@njhill You had provided feedback on the previous PR - would appreciate your review on this one as well.

[njhill]

Thanks @kitaekatt. I benchmarked a low latency case and this also actually appears to have a slight performance benefit 🎉

vllm serve meta-llama/Llama-3.2-1B-Instruct --disable-log-requests --uvicorn-log-level=error --port 8033 --distributed-executor-backend=mp

vllm bench serve \
    --backend vllm \
    --model meta-llama/Llama-3.2-1B-Instruct \
    --dataset-name random \
    --random-input-len 512 \
    --random-output-len 4000 \
    --ignore-eos \
    --num-prompts 12 \
    --max-concurrency 2 \
    --seed 42

Before

============ Serving Benchmark Result ============
Successful requests:                     12        
Failed requests:                         0         
Maximum request concurrency:             2         
Benchmark duration (s):                  49.51     
Total input tokens:                      6132      
Total generated tokens:                  48000     
Request throughput (req/s):              0.24      
Output token throughput (tok/s):         969.52    
Peak output token throughput (tok/s):    1000.00   
Peak concurrent requests:                4.00      
Total Token throughput (tok/s):          1093.38   
---------------Time to First Token----------------
Mean TTFT (ms):                          23.97     
Median TTFT (ms):                        22.70     
P99 TTFT (ms):                           34.61     
-----Time per Output Token (excl. 1st token)------
Mean TPOT (ms):                          2.06      
Median TPOT (ms):                        2.06      
P99 TPOT (ms):                           2.07      
---------------Inter-token Latency----------------
Mean ITL (ms):                           2.06      
Median ITL (ms):                         2.05      
P99 ITL (ms):                            2.37      
==================================================

This PR

============ Serving Benchmark Result ============
Successful requests:                     12        
Failed requests:                         0         
Maximum request concurrency:             2         
Benchmark duration (s):                  47.91     
Total input tokens:                      6132      
Total generated tokens:                  48000     
Request throughput (req/s):              0.25      
Output token throughput (tok/s):         1001.95   
Peak output token throughput (tok/s):    1032.00   
Peak concurrent requests:                4.00      
Total Token throughput (tok/s):          1129.95   
---------------Time to First Token----------------
Mean TTFT (ms):                          16.33     
Median TTFT (ms):                        16.12     
P99 TTFT (ms):                           18.14     
-----Time per Output Token (excl. 1st token)------
Mean TPOT (ms):                          1.99      
Median TPOT (ms):                        1.99      
P99 TPOT (ms):                           2.00      
---------------Inter-token Latency----------------
Mean ITL (ms):                           1.99      
Median ITL (ms):                         1.98      
P99 ITL (ms):                            2.27      
==================================================

[slippersss]

Hi! This looks like an important fix for preventing data races. We've actually encountered similar issues before, as mentioned in #27858, so I really appreciate this effort!

I have a small question about the implementation choice – I was under the impression that threading.Lock works within a single process, while for cross-process shared memory we might need something like multiprocessing.Lock. I'm not entirely sure though, and I'm curious to learn how threading.Lock handles memory visibility across separate processes in this context.

Thanks again for your work on this fix. It's great to see attention to this tricky issue!

[kitaekatt]

Good question @slippersss. For context, this is a lock-free ring buffer - the design specifically avoids blocking so multiple processes can read/write concurrently using atomic flag checks and memory barriers.

The threading.Lock here is being used purely as a memory fence. The shared memory region is already visible across processes via mmap; the issue is ensuring ordering so that Process B observes writes from Process A in the correct sequence.

Acquiring and immediately releasing a threading.Lock forces a full memory barrier at the hardware level (flushes pending writes, invalidates stale cache lines). It's a portable Python idiom since there's no direct mfence wrapper in stdlib.

multiprocessing.Lock would introduce actual blocking between processes, which would negate the lock-free design.

[slippersss]

Good question @slippersss. For context, this is a lock-free ring buffer - the design specifically avoids blocking so multiple processes can read/write concurrently using atomic flag checks and memory barriers.

The threading.Lock here is being used purely as a memory fence. The shared memory region is already visible across processes via mmap; the issue is ensuring ordering so that Process B observes writes from Process A in the correct sequence.

Acquiring and immediately releasing a threading.Lock forces a full memory barrier at the hardware level (flushes pending writes, invalidates stale cache lines). It's a portable Python idiom since there's no direct mfence wrapper in stdlib.

multiprocessing.Lock would introduce actual blocking between processes, which would negate the lock-free design.

Sorry for the late reply, and thanks a lot for the detailed explanation - I learned a lot. Thanks again!