Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 0 additions & 189 deletions .github/workflows/build.yml

This file was deleted.

33 changes: 33 additions & 0 deletions .github/workflows/freethreading_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Free Threading Tests
on: [push, pull_request]
jobs:
tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- name: Grab pythoncapi-compat
run: git clone --depth 1 https://github.com/python/pythoncapi-compat scs/pythoncapi-compat
- name: Install uv and set the python version
uses: astral-sh/setup-uv@v6
with:
python-version: 3.13t
activate-environment: true
enable-cache: false
- name: Set up Python
run: uv python install
- name: Install OpenBLAS
run: |
sudo apt update
sudo apt install -y libopenblas-dev
- name: Install Dependencies
run: |
uv pip install numpy scipy pytest-timeout pytest-durations pytest-run-parallel
- name: Build
run: |
uv pip install -v .
- name: Run Tests
run: |
set -xe
uv run --no-project python -m pytest --parallel-threads=4 --iterations=1 -v -s --timeout=600 --durations=10 -m "not thread_unsafe"
8 changes: 8 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ version = "3.2.7"
description = 'Splitting conic solver'
readme = 'README.md'
requires-python = '>=3.7'
classifiers = [
'Programming Language :: Python :: Free Threading :: 1 - Unstable',
]
license = {file = 'LICENSE'}
authors = [
{name = "Brendan O'Donoghue", email = "[email protected]"}]
Expand Down Expand Up @@ -80,3 +83,8 @@ inherit.before-all = "append"
before-all = [
# "apk update", "apk search -v '*blas*'",
"apk add openblas-dev"]

[tool.pytest.ini_options]
testpaths = [
"test",
]
4 changes: 4 additions & 0 deletions scs/scsmodule.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ static PyObject *moduleinit(void) {
#endif
#endif

#ifdef Py_GIL_DISABLED
PyUnstable_Module_SetGIL(m, Py_MOD_GIL_NOT_USED);
#endif

if (m == NULL) {
return NULL;
}
Expand Down
43 changes: 39 additions & 4 deletions scs/scsobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@
#define PY_SCSOBJECT_H

/* SCS Object type */
#include "pythoncapi-compat/pythoncapi_compat.h"
typedef struct {
PyObject_HEAD ScsWork *work; /* Workspace */
PyObject_HEAD
ScsWork *work; /* Workspace */
ScsSolution *sol; /* Solution, keep around for warm-starts */
scs_int m, n;
#ifdef Py_GIL_DISABLED
PyMutex lock;
#endif
} SCS;

/* Just a helper struct to store the PyArrayObjects that need Py_DECREF */
Expand Down Expand Up @@ -563,10 +568,16 @@ static int SCS_init(SCS *self, PyObject *args, PyObject *kwargs) {
self->sol->s = (scs_float *)scs_calloc(self->m, sizeof(scs_float));

/* release the GIL */
#ifdef Py_GIL_DISABLED

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the GIL protect against the race you're worried about here? If not then you should lock on the GIL-enabled build too. It'd be nice to have an explicit test case for this fix if you don't already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general idea here was to lock before Py_BEGIN_ALLOW_THREADS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since its the initialization call, yes, it is covered by tests :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oops, I completely missed that this is happening inside SCS_init. In that case locking probably isn't necessary, unless it's somehow possible for two threads to simultaneously see the same SCS object before SCS_init returns an initialized object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still a bit unclear about this. The guarantee with the lock is that there can be no two python threads trying to call scs_init (the external library) at the same time...

This is the general pattern of locks around calls to scs.

Under the free threading build without the lock isn't it possible for two threads to try to create scs objects at the same time thereby simultaneously calling into the external library? Or is the point that since the lock is on the object which hasn't been initialized yet it is meaningless anyway (or needs a global lock)?

Copy link

@ngoldbaum ngoldbaum Jun 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to make a distinction between scs_init and SCS_init. The latter is the tp_init slot for SCS_Type - it's the function that we're inside of in this comment thread.

A Python object is effectively thread-local while it's under construction. Said another way, there's no way for another thread to have a handle on the SCS instance before SCS_init completes.

PyMutex_Lock(&self->lock);
#endif
Py_BEGIN_ALLOW_THREADS;
self->work = scs_init(d, k, stgs);
/* reacquire the GIL */
Py_END_ALLOW_THREADS;
#ifdef Py_GIL_DISABLED
PyMutex_Unlock(&self->lock);
#endif

/* no longer need pointers to arrays that held primitives */
free_py_scs_data(d, k, stgs, &ps);
Expand All @@ -582,6 +593,7 @@ static PyObject *SCS_solve(SCS *self, PyObject *args) {
ScsSolution *sol = self->sol;
npy_intp veclen[1];
int scs_float_type = scs_get_float_type();
int errind = -1;

if (!self->work) {
return none_with_error("Workspace not initialized!");
Expand All @@ -606,17 +618,26 @@ static PyObject *SCS_solve(SCS *self, PyObject *args) {
if (_warm_start) {
/* If any of these of missing, we use the values in sol */
if ((void *)warm_x != Py_None) {
if (get_warm_start(self->sol->x, self->n, warm_x) < 0) {
Py_BEGIN_CRITICAL_SECTION(self);
errind = get_warm_start(self->sol->x, self->n, warm_x);
Py_END_CRITICAL_SECTION();
if (errind < 0) {
return none_with_error("Unable to parse x warm-start");
}
}
if ((void *)warm_y != Py_None) {
if (get_warm_start(self->sol->y, self->m, warm_y) < 0) {
Py_BEGIN_CRITICAL_SECTION(self);
errind = get_warm_start(self->sol->y, self->m, warm_y);
Py_END_CRITICAL_SECTION();
if (errind < 0) {
return none_with_error("Unable to parse y warm-start");
}
}
if ((void *)warm_s != Py_None) {
if (get_warm_start(self->sol->s, self->m, warm_s) < 0) {
Py_BEGIN_CRITICAL_SECTION(self);
errind = get_warm_start(self->sol->s, self->m, warm_s);
Py_END_CRITICAL_SECTION();
if (errind < 0) {
return none_with_error("Unable to parse s warm-start");
}
}
Expand All @@ -627,11 +648,17 @@ static PyObject *SCS_solve(SCS *self, PyObject *args) {
PyObject *x, *y, *s, *return_dict, *info_dict;
scs_float *_x, *_y, *_s;
/* release the GIL */
#ifdef Py_GIL_DISABLED
PyMutex_Lock(&self->lock);
#endif
Py_BEGIN_ALLOW_THREADS;
/* Solve! */
scs_solve(self->work, sol, &info, _warm_start);
/* reacquire the GIL */
Py_END_ALLOW_THREADS;
#ifdef Py_GIL_DISABLED
PyMutex_Unlock(&self->lock);
#endif

veclen[0] = self->n;
_x = scs_malloc(self->n * sizeof(scs_float));
Expand Down Expand Up @@ -752,10 +779,16 @@ PyObject *SCS_update(SCS *self, PyObject *args) {
}

/* release the GIL */
#ifdef Py_GIL_DISABLED
PyMutex_Lock(&self->lock);
#endif
Py_BEGIN_ALLOW_THREADS;
scs_update(self->work, b, c);
/* reacquire the GIL */
Py_END_ALLOW_THREADS;
#ifdef Py_GIL_DISABLED
PyMutex_Unlock(&self->lock);
#endif

Py_DECREF(b_new);
Py_DECREF(c_new);
Expand All @@ -766,7 +799,9 @@ PyObject *SCS_update(SCS *self, PyObject *args) {
/* Deallocate SCS object */
static scs_int SCS_finish(SCS *self) {
if (self->work) {
Py_BEGIN_CRITICAL_SECTION(self);
scs_finish(self->work);
Py_END_CRITICAL_SECTION();
}
if (self->sol) {
scs_free(self->sol->x);
Expand Down
1 change: 1 addition & 0 deletions scs/scspy.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "numpy/arrayobject.h" /* Numpy C API */
#include "scs.h" /* SCS API */
#include "scs_types.h" /* SCS primitive types */
#include "pythoncapi-compat/pythoncapi_compat.h"

/* The PyInt variable is a PyLong in Python3.x. */
#if PY_MAJOR_VERSION >= 3
Expand Down
Loading