fix: latent Windows scs_int truncation + OOM crash in SCS_solve#200
Merged
Conversation
Two defensive fixes in SCS_solve's result-building path:
1. Py_BuildValue format mismatch for scs_int under DLONG. The info_dict
format string used 's:l' (long) for scs_int fields, but scs_int is
typedef'd to 'long long' when DLONG is defined (see scs_types.h), and
meson.build sets -DDLONG=1 on all platforms including Windows. On LP64
(Linux/macOS) long == long long, so this is benign; on LLP64 (Windows)
long is 32 bits and would truncate iter counts / status codes. Switch
to 's:L' to match scs_int's actual size — mirrors argparse_string in
SCS_init, which already used 'L' with the same rationale.
2. OOM crash + leak in numpy array construction. After copying sol->{x,y,s}
into heap buffers, we passed each to PyArray_SimpleNewFromData and
unconditionally called PyArray_ENABLEFLAGS on the return. If the numpy
call fails (OOM), it sets a Python exception but does not take
ownership of the buffer — so ENABLEFLAGS would dereference NULL and
crash, plus all three raw buffers would leak. Check each return,
free the un-owned raw buffer, Py_DECREF any already-constructed arrays
(which own their buffers via NPY_ARRAY_OWNDATA), and propagate NULL.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.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.
Summary
Two defensive fixes in
SCS_solve's result-building path inscs/scsobject.h:1.
Py_BuildValueformat mismatch forscs_intunderDLONGThe
info_dictformat string useds:l(long) forscs_intfields, butscs_intis typedef'd tolong longwhenDLONGis defined (seescs_source/include/scs_types.h), andmeson.buildsets-DDLONG=1on every platform including Windows.long == long long == 64 bits, so this is benign today.longis 32 bits whilelong longis 64 bits. Anyscs_intvalue that exceeds 32 bits (iter counts, status codes, etc.) would silently truncate once we build wheels on Windows.Switched to
s:Lto matchscs_int's actual size. This mirrorsargparse_stringinSCS_init, which already used'L'with the same rationale — added a comment cross-referencing it.2. OOM crash + leak in numpy array construction
After copying
sol->{x, y, s}into heap buffers (_x,_y,_s), we passed each toPyArray_SimpleNewFromDataand unconditionally calledPyArray_ENABLEFLAGSon the return. If the numpy call fails (OOM), it sets a Python exception but does not take ownership of the buffer — so:PyArray_ENABLEFLAGSwould dereferenceNULL→ crash.scs_malloc'd buffers would leak.Now each return is NULL-checked: on failure we
scs_freethe still-un-owned raw buffer,Py_DECREFany already-constructed arrays (which own their buffers viaNPY_ARRAY_OWNDATA), and propagateNULL.Test plan
327 passed, 66 skipped)🤖 Generated with Claude Code