Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-116738: Make _json module safe in the free-threading build #119438

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions Include/internal/pycore_critical_section.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ extern "C" {
} \
}

# define Py_RETURN_CRITICAL_SECTION_SEQUENCE_FAST(value) \
if (_should_lock_cs) { \
_PyCriticalSection_End(&_cs); \
} \
return value;

// Asserts that the mutex is locked. The mutex must be held by the
// top-most critical section otherwise there's the possibility
// that the mutex would be swalled out in some code paths.
Expand Down Expand Up @@ -159,6 +165,7 @@ extern "C" {
# define Py_END_CRITICAL_SECTION2()
# define Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(original)
# define Py_END_CRITICAL_SECTION_SEQUENCE_FAST()
# define Py_RETURN_CRITICAL_SECTION_SEQUENCE_FAST(value)
eendebakpt marked this conversation as resolved.
Show resolved Hide resolved
# define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex)
# define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
#endif /* !Py_GIL_DISABLED */
Expand Down
14 changes: 11 additions & 3 deletions Modules/_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "Python.h"
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
#include "pycore_runtime.h" // _PyRuntime

#include "pycore_global_strings.h" // _Py_ID()
Expand Down Expand Up @@ -1615,12 +1616,14 @@

} else {
Py_ssize_t pos = 0;
Py_BEGIN_CRITICAL_SECTION(dct);
while (PyDict_Next(dct, &pos, &key, &value)) {
if (encoder_encode_key_value(s, writer, &first, key, value,
new_newline_indent,
current_item_separator) < 0)
goto bail;
}
Py_END_CRITICAL_SECTION();
}

if (ident != NULL) {
Expand Down Expand Up @@ -1663,9 +1666,13 @@
s_fast = PySequence_Fast(seq, "_iterencode_list needs a sequence");
if (s_fast == NULL)
return -1;

Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(seq);

if (PySequence_Fast_GET_SIZE(s_fast) == 0) {
Py_DECREF(s_fast);
return _PyUnicodeWriter_WriteASCIIString(writer, "[]", 2);
int r = _PyUnicodeWriter_WriteASCIIString(writer, "[]", 2);

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Address sanitizer

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Ubuntu SSL tests with OpenSSL (1.1.1w)

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Hypothesis tests on Ubuntu

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Ubuntu SSL tests with OpenSSL (3.0.13)

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Ubuntu SSL tests with OpenSSL (3.1.5)

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Ubuntu SSL tests with OpenSSL (3.2.1)

unused variable ‘r’ [-Wunused-variable]

Check warning on line 1674 in Modules/_json.c

View workflow job for this annotation

GitHub Actions / Ubuntu / build and test

unused variable ‘r’ [-Wunused-variable]
Py_RETURN_CRITICAL_SECTION_SEQUENCE_FAST(r);
}

if (s->markers != Py_None) {
Expand Down Expand Up @@ -1718,7 +1725,6 @@
goto bail;
Py_CLEAR(ident);
}
eendebakpt marked this conversation as resolved.
Show resolved Hide resolved

if (s->indent != Py_None) {
Py_CLEAR(new_newline_indent);
Py_CLEAR(separator_indent);
Expand All @@ -1730,14 +1736,16 @@
if (_PyUnicodeWriter_WriteChar(writer, ']'))
goto bail;
Py_DECREF(s_fast);
return 0;
Py_RETURN_CRITICAL_SECTION_SEQUENCE_FAST(0);

bail:
Py_END_CRITICAL_SECTION_SEQUENCE_FAST();
Py_XDECREF(ident);
Py_DECREF(s_fast);
Py_XDECREF(separator_indent);
Py_XDECREF(new_newline_indent);
return -1;

eendebakpt marked this conversation as resolved.
Show resolved Hide resolved
}

static void
Expand Down
Loading