indygreg / indygreg/python-zstandard

GIL held during long-running ZSTD operations: 7 sites (5 EOF compression finalizations + dictionary creation + dict-chain content-size)

Open
#300 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
642
Forks
116
Avg merge
1d 14h
Merged PRs (30d)
5

Description

## Summary

7 ZSTD API calls that can run for nontrivial time (milliseconds to seconds on large inputs) are executed with the GIL held. In five of these — all on the compression side, at the `ZSTD_e_end` finalization step — the non-EOF path in the same function correctly wraps `ZSTD_compressStream2` in `Py_BEGIN/END_ALLOW_THREADS`; the EOF finalization step does not. Two more sites (dictionary creation and dict-chain content-size lookup) are additional minor gaps.

Impact is that other Python threads block during these calls, reducing the benefit of running zstandard in a multi-threaded program.

## Impact

- **Severity:** Performance — other threads blocked. No crash, no correctness issue.
- **Reachability:** Multi-threaded programs using zstandard alongside other work; most visible on large compression finalization calls.
- **Version:** 0.25.0 (commit `7a77a75`).

## Sites

### EOF finalization — 5 sites

These finalize a compression stream with `ZSTD_e_end`. For large pending buffers this can take substantial time; the non-EOF loop in the same function already correctly releases the GIL, so the EOF step is an inconsistency.

| File | Line | Context |
|------|------|---------|
| `c-ext/compressoriterator.c` | 129 | `ZstdCompressorIterator` EOF flush |
| `c-ext/compressionreader.c` | 312 | `read()` EOF |
| `c-ext/compressionreader.c` | 444 | `readinto()` EOF |
| `c-ext/compressionreader.c` | 548 | `readall()` EOF |
| `c-ext/compressionreader.c` | 610 | `read1()` EOF |

### Dictionary creation — 1 site

`ZSTD_createCDict_advanced` in `c-ext/compressiondict.c`. Can be slow for large dictionaries (megabytes-plus).

### Dict-chain content-size — 1 site

`ZSTD_getFrameContentSize` in `decompress_content_dict_chain`. Fast per-call but inconsistent with the surrounding code that does release the GIL around the main decompression steps.

## Fix

Wrap each call:

```c
Py_BEGIN_ALLOW_THREADS
zresult = ZSTD_compressStream2(cctx, &output, &input, ZSTD_e_end);
Py_END_ALLOW_THREADS
```

For the five EOF sites, the non-EOF path in the same function already uses this wrapping — consistency is the cleanest way to fix the bug and prevents the GIL-unsafe EOF variant from being reintroduced.

## Suggested PR shape

One PR covering all 7 sites. No behavioral change beyond "other threads can run during these calls". No API surface change.

## Methodology

Found via [cext-review-toolkit](https://github.com/devdanzin/cext-review-toolkit) (Tree-sitter-based static analysis with structured naive/informed review passes). The GIL-discipline scanner identifies ZSTD calls that (a) take a context/cctx pointer that is known to run for nontrivial time and (b) do not sit between `Py_BEGIN_ALLOW_THREADS` / `Py_END_ALLOW_THREADS` macros. The EOF sites were flagged both by the scanner and by the "same function, two paths, only one releases GIL" consistency check in the informed pass. No live reproducer — this is a latent performance issue, not a correctness bug. Happy to open a PR.

*Discovery, root-cause analysis, and issue drafting were performed by [Claude Code](https://www.anthropic.com/claude-code) and reviewed by a human before filing.*

## Full report

Complete multi-agent analysis (48 FIX findings across 13 categories, plus a reproducer appendix): https://gist.github.com/devdanzin/b86039ac097141579590c1a0f3a43605

Contributor guide

Open the contributing guide

Research direction

Start with the five EOF sites in c-ext/compressoriterator.c and c-ext/compressionreader.c, comparing each with its non-EOF path, then inspect ZSTD_createCDict_advanced in c-ext/compressiondict.c and decompress_content_dict_chain. The work is complete when all seven identified ZSTD calls allow other Python threads to run consistently with the surrounding operations, without changing API behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.