Recover from Jython interpreter init failure during constant folding#439
Recover from Jython interpreter init failure during constant folding#439khatchad wants to merge 8 commits intowala:masterfrom
Conversation
`Python3Interpreter.getInterp()` calls `new PythonInterpreter()`, which
walks Jython's bootstrap path to set up `sys.importlib`. In some
environments (e.g., OSGi consumers running under Tycho-surefire) the
bootstrap resources aren't reachable from the current classloader and
the constructor throws `NullPointerException` from
`org.python.core.Py.importSiteIfSelected`. The exception was uncaught —
it propagated through every recursive `ConstantFoldingRewriter.copyNodes`
frame and aborted the entire module load, leaving the class hierarchy
with no `.py` entries.
Fix: catch the failure where it originates, memoize via
`volatile boolean initFailed`, log once at WARNING, and return `null`.
Update `Python3Loader`'s const-fold callback to treat a `null`
interpreter as a folding miss. Memoizing avoids re-running the failing
constructor on every const-fold attempt.
Constant folding is a precision-only feature (it shrinks symbolic
expressions to literals when possible); analysis remains correct
without it, just less precise.
## Reproducer
The downstream symptom is `IllegalStateException: Could not create a
entrypoint callsites:` (with empty `Warnings`) at
`PropagationCallGraphBuilder.makeCallGraph:238`. It surfaces in
Hybridize-Functions-Refactoring's testAutoEncoder /
testTensorFlowEagerExecution / testDatasetIteration4 against any
Ariadne version after `30c15e58` (which removed the silently-swallowing
`try { ... } catch (Throwable e) {}` block around the same code path).
There was a problem hiding this comment.
Pull request overview
This PR makes Jython-based constant folding resilient to Jython interpreter bootstrap failures so Python module loading can proceed (with reduced precision) instead of aborting the entire analysis pipeline.
Changes:
- Memoize Jython interpreter initialization failure in
Python3Interpreter.getInterp()and returnnullon subsequent calls, logging once. - Update
Python3Loader’s constant-folding eval callback to treat anullinterpreter as a folding miss.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/util/Python3Interpreter.java |
Adds failure memoization + logging; changes getInterp() to return null on init failure. |
jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/loader/Python3Loader.java |
Skips constant folding when getInterp() returns null. |
…Integer` Three concerns raised by the auto-review on the previous commit: 1. **Thread-safety**: `initFailed` and `interp` were checked/assigned without synchronization. Make `getInterp()` `synchronized` so the check and the constructor run as a unit through the static class monitor. 2. **`Throwable` is too broad**: catching `Throwable` swallows `Error` types (OOM, StackOverflow, LinkageError) and silently continues with `initFailed=true`, hiding genuine VM problems. Narrow to `Exception`. The Jython failure we're defending against is `RuntimeException` (NPE from `Py.importSiteIfSelected`), which is `Exception`'s subset, so the defensive intent still holds. 3. **`evalAsInteger` didn't null-check `getInterp()`**: with the new contract that `getInterp()` may return null after a memoized init failure, `evalAsInteger().eval(...)` would NPE. Add an `IllegalStateException` for that case so callers get a clear "interpreter unavailable" signal. No behavior change on the success path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror of the fix-branch change applied to the fork PR (per review feedback on #191). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eger` Mirror of the fork-side change (per review on #191). The previous `IllegalStateException` would abort callers that expect the nullable-`Integer` contract; null lets them degrade gracefully. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror of the fork-side polish (per #191). Three items: 1. Memoize per-call WARNING in `evalAsInteger` (first call WARNING, subsequent FINE). 2. Broaden init-failure message to mention all interpreter-based evaluation, not just const-fold. 3. Move `logger.info("Evaluating: ...")` after the `getInterp()` null check in `Python3Loader.eval`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
One note here, @msridhar @juliandolby, the ML code is still not using JEP. So, although @juliandolby transitioned the core modules to JEP, the ML modules still use Jython for the moment. |
There was a problem hiding this comment.
Pull request overview
This PR makes constant folding and interpreter-based evaluation resilient to environment-dependent Jython initialization failures, so Python module loading (and downstream call graph construction) can proceed even when new PythonInterpreter() cannot be created (e.g., under Tycho/OSGi).
Changes:
- Add failure memoization to
Python3Interpreter.getInterp()so a Jython init failure is logged once and subsequent calls returnnullcheaply. - Update
Python3Loaderconstant folding evaluation to treat anullinterpreter as a folding miss (returnnull) rather than aborting module load.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/util/Python3Interpreter.java | Memoize interpreter init failure, avoid repeated expensive init attempts, and degrade evalAsInteger behavior when interpreter is unavailable. |
| jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/loader/Python3Loader.java | Skip constant folding evaluation when the interpreter is unavailable, preventing module-load aborts due to init failures. |
Comments suppressed due to low confidence (2)
jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/util/Python3Interpreter.java:100
evalAsInteger()can still throwIllegalArgumentExceptionwhen the expression isn’t an integer or when Jython evaluation raisesPyException. Call sites likePythonInterpreter.interpretAsInt(expr)(e.g., inTensorType.shapeArgandPythonTurtleAnalysisEngine) don’t catch and instead rely on anullreturn to fall back gracefully, so these exceptions can still abort analysis on non-constant/invalid expressions. Consider returningnullfor “cannot evaluate” cases (and logging at FINE/WARNING as desired), or updating the call sites to handle the exception explicitly.
try {
PyObject val = ip.eval(expr);
if (val.isInteger()) {
return val.asInt();
} else
throw new IllegalArgumentException(
"Python expression: " + expr + " cannot be evaluated to an integer.");
} catch (PyException e) {
LOGGER.log(Level.SEVERE, "Unable to interpret Python expression: " + expr, e);
throw new IllegalArgumentException("Can't interpret Python expression: " + expr + ".", e);
}
jython/com.ibm.wala.cast.python.jython3/source/com/ibm/wala/cast/python/loader/Python3Loader.java:127
eval(...)only catchesPySyntaxError. Other runtime evaluation failures from Jython (e.g.,PyExceptionlikeZeroDivisionError,TypeError, etc.) will propagate out of constant folding and can still abort module loading/call graph construction, even though constant folding is intended to be best-effort. Consider catching broader evaluation exceptions here (at leastorg.python.core.PyException, orExceptionlike the Python2Loader implementation) and returningnullto treat it as a folding miss (optionally log at FINE).
try {
x = ip.eval(unicode);
} catch (PySyntaxError e) {
// Handle syntax errors gracefully.
logger.log(WARNING, e, () -> "Syntax error in expression: " + unicode);
return null;
}
…lableWarned` Mirror of the fork-side fix (per #191). The previous `if (!unavailableWarned) { unavailableWarned = true; ... }` sequence isn't atomic; switch to `AtomicBoolean.compareAndSet(false, true)` so only the thread that flips the flag enters the WARNING branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror of the fork-side change (per #191 review feedback). The "uses AtomicBoolean.compareAndSet for atomicity" prose is an implementation detail, not contract; move it under `@implNote`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The failing #443 removes that workflow step (root cause documented in #433). Once #443 lands and this branch is rebased on |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #439 +/- ##
============================================
- Coverage 57.91% 57.82% -0.10%
Complexity 624 624
============================================
Files 111 111
Lines 7671 7690 +19
Branches 856 860 +4
============================================
+ Hits 4443 4447 +4
- Misses 3050 3063 +13
- Partials 178 180 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Two-file change to keep module load alive when
new PythonInterpreter()fails during constant folding. The currentPython3Interpreter.getInterp()propagates the failure through every recursiveConstantFoldingRewriter.copyNodesframe and aborts the entire module load, leaving the class hierarchy with no.pyentries andmakeDefaultEntrypointsreturning empty.Symptom
The downstream symptom is
IllegalStateException: Could not create a entrypoint callsites:(with emptyWarnings) atPropagationCallGraphBuilder.makeCallGraph:238, because the failed module load leaves the CHA without any.pyentries.Why It Surfaces Now
Both branches in this commit-pair were silently swallowing the failure prior to a recent change in the
ponder-lab/MLfork: atry { ... } catch (Throwable e) {}block ingetInterp()was removed, andPySystemState.initialize()(previously commented out) was uncommented. Removing the silent swallow exposed an env-dependent Jython bootstrap failure to consumers whose classloader/working-directory setup can't satisfy Jython's_frozen_importliblookup.Change
Two coupled edits, ~35 lines total:
Python3Interpreter.getInterp()— wrapnew PythonInterpreter()in try/catch, log once atWARNING, memoize the failure viavolatile boolean initFailed, returnnullinstead of throwing. Memoizing matters because const-fold is invoked recursively per AST node; on a module with N const-eligible expressions, retrying the failing constructor each time would be O(N) Jython init attempts.Python3Loader'sConstantFoldingRewriter.evalcallback — treat anullinterpreter as a folding miss (returnnull). Analysis remains correct, just less precise — constant folding is a precision-only feature.Validation
mvn -pl com.ibm.wala.cast.python.ml.test -am test(with this fix on the ponder-lab fork): 610 tests, 0 failures, 0 errors, 3 skipped. No regression on the existing test surface.Tycho-OSGi reproducer (Hybridize-Functions-Refactoring
upgrade-tycho-5-java-25branch consuming a SNAPSHOT with this fix): the 3IllegalStateExceptionfailures (testAutoEncoder,testTensorFlowEagerExecution,testDatasetIteration4) no longer surface — const-fold is a no-op for those modules under OSGi but module load proceeds and downstream tensor analysis runs.Branch
This branch is rooted at
wala/ML:masterdirectly (not ponder-lab fork master) so the diff is exactly the two-file change here, with no fork-only commits riding along.