yara-x follow-ups: lint, thread-affinity tests, and a recompile backoff - #3152
Merged
doomedraven merged 4 commits intoAug 9, 2026
Merged
Conversation
pebble.py's 'import sys' went unused when the PanicException stub was removed (F401), and the new engine tests have whitespace on blank lines (W293). The pre-commit ruff hook fails on both.
yara_x.Scanner is unsendable: PyO3 panics if one is touched from a thread other than the one that built it. Nothing covered this, because test_yara_x skips whenever yara-x is not installed -- the default, since pyproject pins yara-python -- so CI never executes the yara-x branch at all. Adds four tests (skipped without yara-x, so CI behaviour is unchanged) and a comment at the cache site recording why the Scanner must not be cached -- including per-thread, since Scanner is unsendable for drop as well as use and a cached one is dropped on the wrong thread on fork and at shutdown.
get_yara() forces a full six-category recompile whenever a category is missing, and remembers nothing -- so a category that cannot produce rules costs one full recompile per scanned file (~3s each on a production ruleset; CAPE.py and procmemory.py call this per extracted payload). Records the failure with a 300s backoff rather than a permanent skip: workers run with max_tasks=0 (no recycling), so disabling a category after one transient failure would silently return no matches for the rest of the run. A forced re-init drops the backoff for categories that actually compiled -- clearing the whole record would make each broken category forget the others and recompile on every alternating call.
There was a problem hiding this comment.
Pull request overview
This PR adds follow-up fixes around the yara-x integration: cleaning up lint issues, adding thread-affinity regression tests for yara_x.Scanner, and preventing repeated expensive YARA recompiles by introducing a per-category recompile backoff in File.get_yara().
Changes:
- Remove a now-unused import and fix whitespace-only ruff violations in tests.
- Add (skipped-by-default) tests to guard against caching/using
yara_x.Scanneracross threads. - Add a per-category backoff to avoid forcing a full YARA recompile on every file when a category can’t produce rules.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_yara_x_thread_affinity.py | Adds yara-x thread-affinity and recompilation/backoff behavior tests (skipped unless yara-x is installed). |
| tests/test_pebble_engine.py | Removes trailing whitespace on blank lines to satisfy ruff. |
| lib/cuckoo/core/processing_engine/pebble.py | Removes an unused sys import to satisfy ruff (F401). |
| lib/cuckoo/common/objects.py | Adds yara_uncompilable backoff tracking and ensures yara-x path constructs scanners on the scanning thread. |
Suppressed comments (2)
tests/test_yara_x_thread_affinity.py:123
- Same as above: the test setup creates a
scriptsdirectory, but the real code expectsmonitoras the 6th built-in category. Aligning the category list avoids noisy warnings and ensures this test exercises the actual init_yara() directory layout.
for category in ("binaries", "urls", "memory", "scripts", "macro", "CAPE"):
for tree in ("data", "custom"):
(tmp_path / tree / "yara" / category).mkdir(parents=True, exist_ok=True)
tests/test_yara_x_thread_affinity.py:187
- Same category-list mismatch here:
File.init_yara()usesmonitor, notscripts. Using the real category list preventsMissing Yara directory: .../monitorwarnings during this test run.
for category in ("binaries", "urls", "memory", "scripts", "macro", "CAPE"):
for tree in ("data", "custom"):
(tmp_path / tree / "yara" / category).mkdir(parents=True, exist_ok=True)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The tests created a 'scripts' YARA directory, but init_yara() compiles 'monitor' -- so the real category was never exercised and every run logged a missing-directory warning for it. Use the actual category tuple. Snapshot cls.yara_rules before iterating it: it is class-level and two threads can be inside a forced init at once (get_yara's fallback triggers one), so iterating it directly risks 'dictionary changed size during iteration'. Snapshotting also keeps categories injected outside the built-in list, which iterating the fixed tuple would miss.
doomedraven
added a commit
that referenced
this pull request
Aug 9, 2026
* Implement workaround for PanicException serialization Added a workaround for PanicException serialization failures by dynamically registering a module. This allows for successful pickle/unpickle operations. * Will's fixes + some extra * fix yara-x * yara-x follow-ups: lint, thread-affinity tests, and a recompile backoff (#3152) * lint: fix ruff failures on this branch pebble.py's 'import sys' went unused when the PanicException stub was removed (F401), and the new engine tests have whitespace on blank lines (W293). The pre-commit ruff hook fails on both. * tests: guard the yara-x Scanner thread-affinity fix yara_x.Scanner is unsendable: PyO3 panics if one is touched from a thread other than the one that built it. Nothing covered this, because test_yara_x skips whenever yara-x is not installed -- the default, since pyproject pins yara-python -- so CI never executes the yara-x branch at all. Adds four tests (skipped without yara-x, so CI behaviour is unchanged) and a comment at the cache site recording why the Scanner must not be cached -- including per-thread, since Scanner is unsendable for drop as well as use and a cached one is dropped on the wrong thread on fork and at shutdown. * objects: back off instead of recompiling YARA rules for every file get_yara() forces a full six-category recompile whenever a category is missing, and remembers nothing -- so a category that cannot produce rules costs one full recompile per scanned file (~3s each on a production ruleset; CAPE.py and procmemory.py call this per extracted payload). Records the failure with a 300s backoff rather than a permanent skip: workers run with max_tasks=0 (no recycling), so disabling a category after one transient failure would silently return no matches for the rest of the run. A forced re-init drops the backoff for categories that actually compiled -- clearing the whole record would make each broken category forget the others and recompile on every alternating call. * Address review: real category list in tests, snapshot before iterating The tests created a 'scripts' YARA directory, but init_yara() compiles 'monitor' -- so the real category was never exercised and every run logged a missing-directory warning for it. Use the actual category tuple. Snapshot cls.yara_rules before iterating it: it is class-level and two threads can be inside a forced init at once (get_yara's fallback triggers one), so iterating it directly risks 'dictionary changed size during iteration'. Snapshotting also keeps categories injected outside the built-in list, which iterating the fixed tuple would miss. --------- Co-authored-by: William Metcalf <wmetcalf@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups to #3142, targeting that PR's branch so they land with it rather than drifting into a cleanup that never happens. All three came out of chasing the
PanicExceptionin #3142.Three independent commits — drop any of them freely:
lint: fix ruff failures on this branch—pebble.py'simport syswent unused when the stub was removed (F401), and the new engine tests have whitespace on blank lines (W293). The pre-commit ruff hook currently fails on both.tests: guard the yara-x Scanner thread-affinity fix—yara_x.Scanneris unsendable; PyO3 panics if one is touched from a thread other than the one that built it. Nothing covered this, becausetest_yara_xskips whenever yara-x isn't installed — the default, sincepyproject.tomlpinsyara-python— so CI never executes the yara-x branch at all. Four tests (skipped without yara-x, so CI behaviour is unchanged), plus a comment at the cache site recording why the Scanner must not be cached including per-thread: it's unsendable for drop as well as use, so a cached one gets dropped on the wrong thread onforkand at shutdown, printingRuntimeError: yara_x::Scanner is unsendable, but is being dropped on another thread. I tried that optimisation and backed it out; the comment is there so the next person doesn't repeat it.objects: back off instead of recompiling YARA rules for every file— pre-existing, surfaced while tracing the yara-x path.get_yara()forces a full six-category recompile whenever a category is missing and remembers nothing, so a category that can't produce rules costs one full recompile per scanned file (~3s each on a production ruleset;CAPE.pyandprocmemory.pycall this per extracted payload). Measured on the repo's own 115 rule files: three calls → three forced recompiles at ~0.1s each.The obvious fix — remember the failure and skip — is a trap. Workers run with
max_tasks=0(no recycling), so a transient failure (rules mid-update, storage briefly unreadable) would silently return[]for that whole category for the rest of the run. Missed detections beat wasted CPU as a failure mode, so it's a 300s backoff instead. A forced re-init drops the backoff for categories that actually compiled — clearing the whole record would make each broken category forget the others and recompile on every alternating call.Verified on
e477564— each commit independently: ruff and isort clean at every step, new tests 4 then 9 passing with yara-x installed and skipped without it, full suite 678 passed / 48 skipped / 1 failed (test_mitre, which fails identically on a clean checkout).🤖 Generated with Claude Code