ITADN

Thread-safety: data race in ImagingDefaultArena block cache (memory_get_block / memory_return_block)

#9600Openctkhanhly 创建于 2026-05-01
C
ctkhanhlycommented
### What did you do? Used `asyncio.to_thread()` / `ThreadPoolExecutor` to offload PIL Image operations (create, load, resize, encode) to multiple threads concurrently. ### What did you expect to happen? Thread-safe access to PIL Image allocation/deallocation, or documentation stating that multi-threaded usage requires `set_blocks_max(0)`. ### What actually happened? `ImagingDefaultArena` in `src/libImaging/Storage.c` uses shared mutable state (`blocks_cached`, `blocks_pool`) without any synchronization. When multiple threads call `memory_get_block` / `memory_return_block` concurrently (which happens when PIL's GIL-releasing C operations run in parallel), data races can occur: ```c // memory_get_block (line 310) — no lock if (arena->blocks_cached > 0) { arena->blocks_cached -= 1; // ← read-modify-write, no lock block = arena->blocks_pool[arena->blocks_cached]; // ← concurrent read, no lock } // memory_return_block (line 347) — no lock if (arena->blocks_cached < arena->blocks_max) { arena->blocks_pool[arena->blocks_cached] = block; // ← concurrent write, no lock arena->blocks_cached += 1; // ← read-modify-write, no lock } else { free(block.ptr); } ``` **Race scenario** (two threads returning blocks simultaneously): ``` Thread A: reads blocks_cached = 19 (< blocks_max 20) Thread B: reads blocks_cached = 19 (same stale value — no lock) Thread A: writes blocks_pool[19] = block_A, increments to 20 Thread B: writes blocks_pool[19] = block_B (OVERWRITES block_A) Result: block_A.ptr is permanently lost — malloc'd, never free'd. ``` Similarly, `memory_get_block` can hand the same cached block to two threads simultaneously (use-after-free / double-free risk). The race window is narrow under the GIL (requires concurrent C-level execution during GIL-released operations like resize/encode), making it hard to reproduce in simple tests. Under sustained high-concurrency production workloads with longer GIL-released operations, the race triggers more frequently. **Workaround:** `Image.core.set_blocks_max(0)` disables the cache entirely, eliminating the shared mutable state. ### What are your OS, Python and Pillow versions? * OS: Linux x86_64 * Python: 3.12 * Pillow: 10.4.0 ```python import asyncio from concurrent.futures import ThreadPoolExecutor from PIL import Image # This triggers concurrent access to ImagingDefaultArena from multiple threads. # The race is probabilistic and more likely under sustained production load # with longer GIL-released operations (resize, encode). async def main(): loop = asyncio.get_event_loop() loop.set_default_executor(ThreadPoolExecutor(max_workers=20)) resolutions = [(512, 512), (1024, 1024), (1536, 1536), (2048, 2048)] async def process(res): def _work(): img = Image.new("RGB", res) img.load() await asyncio.to_thread(_work) for _ in range(1000): await asyncio.gather(*[process(resolutions[i % 4]) for i in range(20)]) asyncio.run(main()) ``` **Additional context:** PIL's C extensions release the GIL during image operations, enabling true parallelism in thread pools. However, `ImagingDefaultArena` is a process-global struct with no mutex or atomic operations protecting its state. When `asyncio.to_thread` dispatches PIL work to a `ThreadPoolExecutor`, multiple threads can call `memory_get_block` / `memory_return_block` concurrently during the GIL-released portions of operations like `Image.resize()` or `Image.save()`. This is particularly relevant for: - Python's increasing use of `asyncio.to_thread` for GIL-releasing C extensions - The upcoming free-threaded CPython (PEP 703), which will remove the GIL entirely and make this race trivially triggerable **Suggested fixes:** 1. Add a `pthread_mutex` around `blocks_cached` / `blocks_pool` access in `memory_get_block` and `memory_return_block` 2. Or use per-thread arenas (eliminates contention) 3. Or document that the block cache is not thread-safe and recommend `set_blocks_max(0)` for multi-threaded usage
1 条评论