Skip to content

Commit fb2d286

Browse files
Address Copilot review comments on PR #62
- fastparser.py: replace the shared matching/meanings cache dicts instead of clearing them in place when the size cap is hit; in-flight parse jobs in other threads hold references to the current dicts, which keeps the CFFI buffers that the C++ core points into alive until those jobs complete (avoiding a potential use-after-free) - eparser.h/.cpp: make the parity mismatch counter a relaxed atomic, consistent with the other counters, since it is incremented from within concurrent parses - CLAUDE.md: update stale guidance - mypy is now clean and runs as a CI gate on non-PyPy jobs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4a846b8 commit fb2d286

4 files changed

Lines changed: 15 additions & 9 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ uv run pytest test/test_parse.py::test_long_parse
2929
# Lint (CI runs this)
3030
uv run ruff check src/reynir
3131

32-
# Type check (config in pyproject.toml [tool.mypy]; carries a handful of
33-
# known pre-existing errors, so it is not a CI gate)
32+
# Type check (config in pyproject.toml [tool.mypy]; runs as a CI gate
33+
# on non-PyPy jobs, so it must stay clean)
3434
uv run mypy src/reynir
3535
```
3636

src/reynir/eparser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ void Parser::setMatchingTable(const BYTE* pSpecs, UINT nSpecs,
11021102
this->m_nSpecs = 0;
11031103
this->m_pMeaningsFunc = NULL;
11041104
this->m_bParity = bParity;
1105-
this->m_nParityMismatches = 0;
1105+
this->m_nParityMismatches.store(0, std::memory_order_relaxed);
11061106
memset(&this->m_masks, 0, sizeof(MatchMasks));
11071107
if (!pSpecs || !nSpecs || !fpMeanings || !pMasks)
11081108
return;

src/reynir/eparser.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,9 @@ class Parser {
422422
MeaningsFunc m_pMeaningsFunc;
423423
MatchMasks m_masks;
424424
BOOL m_bParity; // Parity checking mode
425-
UINT m_nParityMismatches;
425+
// Atomic (like the diagnostic allocation counters), since it is
426+
// incremented from within concurrent parses
427+
std::atomic<UINT> m_nParityMismatches;
426428

427429
void push(UINT nHandle, State*, Column*, State*&, StateChunk*);
428430

@@ -466,9 +468,9 @@ class Parser {
466468
BOOL parityMode(void) const
467469
{ return this->m_bParity; }
468470
void countParityMismatch(void)
469-
{ this->m_nParityMismatches++; }
471+
{ this->m_nParityMismatches.fetch_add(1, std::memory_order_relaxed); }
470472
UINT getParityMismatches(void) const
471-
{ return this->m_nParityMismatches; }
473+
{ return this->m_nParityMismatches.load(std::memory_order_relaxed); }
472474

473475
// Evaluate a native token/terminal match
474476
static BOOL evalMatch(const TerminalSpec*, const BYTE* pTokenRec, const MatchMasks&);

src/reynir/fastparser.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -881,11 +881,15 @@ def go(self, tokens: Iterable[Tok], *, root: Optional[str] = None) -> Node:
881881
result: Optional[Node] = None
882882

883883
if len(self._matching_cache) > self._MAX_MATCHING_CACHE_SIZE:
884-
# The matching cache has grown too large: clear it.
884+
# The matching cache has grown too large: discard it.
885885
# The cost is only that subsequent parses need to re-match
886886
# tokens against terminals as they are encountered again.
887-
self._matching_cache.clear()
888-
self._meanings_cache.clear()
887+
# Note: the dicts are replaced, not cleared in place. Parse
888+
# jobs that may be in flight in other threads hold references
889+
# to the current dicts, which keeps the CFFI buffers that the
890+
# C++ core points into alive until those jobs complete.
891+
self._matching_cache = dict()
892+
self._meanings_cache = dict()
889893

890894
# Use the context manager protocol to guarantee that the parse job
891895
# handle will be properly deleted even if an exception is thrown

0 commit comments

Comments
 (0)