Fix vendored PSLP writing a stray infeasible-message printf to native stdout - #1836
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughChangesPSLP dependency update
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change stops an unintended PSLP status message from being written to native stdout, preventing interference with Java test-process communication; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
run_presolver() prints "PSLP declares problem as infeasible[.| or unbounded.]" unconditionally, unlike every other console message in that function, which are all gated on stgs->verbose. cuOpt sets verbose = false when calling PSLP (third_party_presolve.cpp) specifically to keep it silent, so this writes unexpectedly straight to the process's native stdout. Found and confirmed via a captured Surefire dumpstream while investigating the Java bindings' "Corrupted channel" crash (see #1818): a raw native write there bypasses System.out and corrupts Maven Surefire's forked-JVM protocol, which also uses stdout as its own channel. The infeasible/unbounded status itself is unaffected -- it already flows back to the caller through run_presolver()'s typed return value, not by parsing this printed text. Filed and fixed upstream: dance858/PSLP#55. Pin past v0.0.11 to the merge commit until a release containing it is available.
3f7aa35 to
e2d602f
Compare
CI Test Summary✅ All 31 test job(s) passed. |
|
/merge |
Summary
run_presolver()in vendored PSLP (dance858/PSLP, pinned atv0.0.11) prints"PSLP declares problem as infeasible[.| or unbounded.]"unconditionally, unlike every other console message in that function, which are all gated behindstgs->verbose(print_start_message,print_end_message, etc).verbose = falsewhen calling PSLP (third_party_presolve.cpp) specifically to keep it silent -- this one message was just missed upstream, so it writes straight to the process's native stdout regardless.System.outand corrupts Maven Surefire's forked-JVM protocol, which also multiplexes over stdout. Confirmed directly by reading the exact corrupted text out of a captured Surefire.dumpstreamartifact.run_presolver()'s typed return value, not by parsing this printed text.Fix
Filed and fixed upstream: dance858/PSLP#55. Until a release containing it ships, pin
cpp/CMakeLists.txt'sGIT_TAGpastv0.0.11directly at the merge commit, rather than patching the vendored source locally.Test plan
.dumpstreamartifact that this exact message was the corrupted text.ProblemIntegrationTest's infeasible-solve test case (Java bindings) exercises this code path directly.Split out of #1825, which originally carried this alongside an unrelated fix (routing cuOpt's own console logging through
System.out).