From 9a9d9f7399f7b83b6567d1d9dbefb605bb685fd3 Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 15 Nov 2023 23:18:34 +0800 Subject: [PATCH 01/12] Using critical sections to make ``io.StringIO`` thread safe. --- .../pycore_global_objects_fini_generated.h | 1 + Include/internal/pycore_global_strings.h | 1 + .../internal/pycore_runtime_init_generated.h | 1 + .../internal/pycore_unicodeobject_generated.h | 3 + Modules/_io/clinic/stringio.c.h | 152 +++++++++++++++++- Modules/_io/stringio.c | 66 ++++++-- 6 files changed, 200 insertions(+), 24 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 0808076f44de31..e2c248317e8822 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -1190,6 +1190,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(src_dir_fd)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(stacklevel)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(start)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(state)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(statement)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(status)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(stderr)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 8d22a9ba261010..267094357feba1 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -679,6 +679,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(src_dir_fd) STRUCT_FOR_ID(stacklevel) STRUCT_FOR_ID(start) + STRUCT_FOR_ID(state) STRUCT_FOR_ID(statement) STRUCT_FOR_ID(status) STRUCT_FOR_ID(stderr) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index d41a7478db663f..bdc5e008c5790d 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -1188,6 +1188,7 @@ extern "C" { INIT_ID(src_dir_fd), \ INIT_ID(stacklevel), \ INIT_ID(start), \ + INIT_ID(state), \ INIT_ID(statement), \ INIT_ID(status), \ INIT_ID(stderr), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index 0c02e902b308e3..701030c03f46e1 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -1878,6 +1878,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { string = &_Py_ID(start); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); + string = &_Py_ID(state); + assert(_PyUnicode_CheckConsistency(string, 1)); + _PyUnicode_InternInPlace(interp, &string); string = &_Py_ID(statement); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); diff --git a/Modules/_io/clinic/stringio.c.h b/Modules/_io/clinic/stringio.c.h index 571ec5117147d5..9fd116c026e1f1 100644 --- a/Modules/_io/clinic/stringio.c.h +++ b/Modules/_io/clinic/stringio.c.h @@ -24,7 +24,13 @@ _io_StringIO_getvalue_impl(stringio *self); static PyObject * _io_StringIO_getvalue(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_getvalue_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_getvalue_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io_StringIO_tell__doc__, @@ -42,7 +48,13 @@ _io_StringIO_tell_impl(stringio *self); static PyObject * _io_StringIO_tell(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_tell_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_tell_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io_StringIO_read__doc__, @@ -76,7 +88,9 @@ _io_StringIO_read(stringio *self, PyObject *const *args, Py_ssize_t nargs) goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _io_StringIO_read_impl(self, size); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -112,7 +126,9 @@ _io_StringIO_readline(stringio *self, PyObject *const *args, Py_ssize_t nargs) goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _io_StringIO_readline_impl(self, size); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -150,7 +166,9 @@ _io_StringIO_truncate(stringio *self, PyObject *const *args, Py_ssize_t nargs) goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _io_StringIO_truncate_impl(self, size); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -204,7 +222,9 @@ _io_StringIO_seek(stringio *self, PyObject *const *args, Py_ssize_t nargs) goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _io_StringIO_seek_impl(self, pos, whence); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -222,6 +242,21 @@ PyDoc_STRVAR(_io_StringIO_write__doc__, #define _IO_STRINGIO_WRITE_METHODDEF \ {"write", (PyCFunction)_io_StringIO_write, METH_O, _io_StringIO_write__doc__}, +static PyObject * +_io_StringIO_write_impl(stringio *self, PyObject *obj); + +static PyObject * +_io_StringIO_write(stringio *self, PyObject *obj) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_write_impl(self, obj); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_io_StringIO_close__doc__, "close($self, /)\n" "--\n" @@ -242,7 +277,13 @@ _io_StringIO_close_impl(stringio *self); static PyObject * _io_StringIO_close(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_close_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_close_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io_StringIO___init____doc__, @@ -330,7 +371,13 @@ _io_StringIO_readable_impl(stringio *self); static PyObject * _io_StringIO_readable(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_readable_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_readable_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io_StringIO_writable__doc__, @@ -348,7 +395,13 @@ _io_StringIO_writable_impl(stringio *self); static PyObject * _io_StringIO_writable(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_writable_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_writable_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io_StringIO_seekable__doc__, @@ -366,6 +419,91 @@ _io_StringIO_seekable_impl(stringio *self); static PyObject * _io_StringIO_seekable(stringio *self, PyObject *Py_UNUSED(ignored)) { - return _io_StringIO_seekable_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO_seekable_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +PyDoc_STRVAR(_io_StringIO___getstate____doc__, +"__getstate__($self, /)\n" +"--\n" +"\n"); + +#define _IO_STRINGIO___GETSTATE___METHODDEF \ + {"__getstate__", (PyCFunction)_io_StringIO___getstate__, METH_NOARGS, _io_StringIO___getstate____doc__}, + +static PyObject * +_io_StringIO___getstate___impl(stringio *self); + +static PyObject * +_io_StringIO___getstate__(stringio *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO___getstate___impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +PyDoc_STRVAR(_io_StringIO___setstate____doc__, +"__setstate__($self, /, state)\n" +"--\n" +"\n"); + +#define _IO_STRINGIO___SETSTATE___METHODDEF \ + {"__setstate__", _PyCFunction_CAST(_io_StringIO___setstate__), METH_FASTCALL|METH_KEYWORDS, _io_StringIO___setstate____doc__}, + +static PyObject * +_io_StringIO___setstate___impl(stringio *self, PyObject *state); + +static PyObject * +_io_StringIO___setstate__(stringio *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 1 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { &_Py_ID(state), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"state", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "__setstate__", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[1]; + PyObject *state; + + args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, argsbuf); + if (!args) { + goto exit; + } + state = args[0]; + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io_StringIO___setstate___impl(self, state); + Py_END_CRITICAL_SECTION(); + +exit: + return return_value; } -/*[clinic end generated code: output=f56aa7f8a271acf6 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=dedaeb21e1ed20c6 input=a9049054013a1b77]*/ diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 1856b07108bab6..cc4a56be3e7fc3 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -1,6 +1,7 @@ #include "Python.h" #include // offsetof() #include "pycore_object.h" +#include "pycore_critical_section.h" #include "_iomodule.h" /* Implementation note: the buffer is always at least one character longer @@ -263,6 +264,7 @@ write_str(stringio *self, PyObject *obj) } /*[clinic input] +@critical_section _io.StringIO.getvalue Retrieve the entire contents of the object. @@ -270,7 +272,7 @@ Retrieve the entire contents of the object. static PyObject * _io_StringIO_getvalue_impl(stringio *self) -/*[clinic end generated code: output=27b6a7bfeaebce01 input=d23cb81d6791cf88]*/ +/*[clinic end generated code: output=27b6a7bfeaebce01 input=fb5dee06b8d467f3]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -281,6 +283,7 @@ _io_StringIO_getvalue_impl(stringio *self) } /*[clinic input] +@critical_section _io.StringIO.tell Tell the current file position. @@ -288,7 +291,7 @@ Tell the current file position. static PyObject * _io_StringIO_tell_impl(stringio *self) -/*[clinic end generated code: output=2e87ac67b116c77b input=ec866ebaff02f405]*/ +/*[clinic end generated code: output=2e87ac67b116c77b input=98a08f3e2dae3550]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -296,6 +299,7 @@ _io_StringIO_tell_impl(stringio *self) } /*[clinic input] +@critical_section _io.StringIO.read size: Py_ssize_t(accept={int, NoneType}) = -1 / @@ -308,7 +312,7 @@ is reached. Return an empty string at EOF. static PyObject * _io_StringIO_read_impl(stringio *self, Py_ssize_t size) -/*[clinic end generated code: output=ae8cf6002f71626c input=0921093383dfb92d]*/ +/*[clinic end generated code: output=ae8cf6002f71626c input=9fbef45d8aece8e7]*/ { Py_ssize_t n; Py_UCS4 *output; @@ -368,6 +372,7 @@ _stringio_readline(stringio *self, Py_ssize_t limit) } /*[clinic input] +@critical_section _io.StringIO.readline size: Py_ssize_t(accept={int, NoneType}) = -1 / @@ -379,7 +384,7 @@ Returns an empty string if EOF is hit immediately. static PyObject * _io_StringIO_readline_impl(stringio *self, Py_ssize_t size) -/*[clinic end generated code: output=cabd6452f1b7e85d input=a5bd70bf682aa276]*/ +/*[clinic end generated code: output=cabd6452f1b7e85d input=4d14b8495dea1d98]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -427,6 +432,7 @@ stringio_iternext(stringio *self) } /*[clinic input] +@critical_section _io.StringIO.truncate pos as size: Py_ssize_t(accept={int, NoneType}, c_default="self->pos") = None / @@ -440,7 +446,7 @@ Returns the new absolute position. static PyObject * _io_StringIO_truncate_impl(stringio *self, Py_ssize_t size) -/*[clinic end generated code: output=eb3aef8e06701365 input=5505cff90ca48b96]*/ +/*[clinic end generated code: output=eb3aef8e06701365 input=461b872dce238452]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -462,6 +468,7 @@ _io_StringIO_truncate_impl(stringio *self, Py_ssize_t size) } /*[clinic input] +@critical_section _io.StringIO.seek pos: Py_ssize_t whence: int = 0 @@ -478,7 +485,7 @@ Returns the new absolute position. static PyObject * _io_StringIO_seek_impl(stringio *self, Py_ssize_t pos, int whence) -/*[clinic end generated code: output=e9e0ac9a8ae71c25 input=e3855b24e7cae06a]*/ +/*[clinic end generated code: output=e9e0ac9a8ae71c25 input=c75ced09343a00d7]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -515,6 +522,7 @@ _io_StringIO_seek_impl(stringio *self, Py_ssize_t pos, int whence) } /*[clinic input] +@critical_section _io.StringIO.write s as obj: object / @@ -526,8 +534,8 @@ the length of the string. [clinic start generated code]*/ static PyObject * -_io_StringIO_write(stringio *self, PyObject *obj) -/*[clinic end generated code: output=0deaba91a15b94da input=cf96f3b16586e669]*/ +_io_StringIO_write_impl(stringio *self, PyObject *obj) +/*[clinic end generated code: output=d53b1d841d7db288 input=1561272c0da4651f]*/ { Py_ssize_t size; @@ -547,6 +555,7 @@ _io_StringIO_write(stringio *self, PyObject *obj) } /*[clinic input] +@critical_section _io.StringIO.close Close the IO object. @@ -559,7 +568,7 @@ This method has no effect if the file is already closed. static PyObject * _io_StringIO_close_impl(stringio *self) -/*[clinic end generated code: output=04399355cbe518f1 input=cbc10b45f35d6d46]*/ +/*[clinic end generated code: output=04399355cbe518f1 input=305d19aa29cc40b9]*/ { self->closed = 1; /* Free up some memory */ @@ -756,6 +765,7 @@ _io_StringIO___init___impl(stringio *self, PyObject *value, /* Properties and pseudo-properties */ /*[clinic input] +@critical_section _io.StringIO.readable Returns True if the IO object can be read. @@ -763,7 +773,7 @@ Returns True if the IO object can be read. static PyObject * _io_StringIO_readable_impl(stringio *self) -/*[clinic end generated code: output=b19d44dd8b1ceb99 input=39ce068b224c21ad]*/ +/*[clinic end generated code: output=b19d44dd8b1ceb99 input=6cd2ffd65a8e8763]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -771,6 +781,7 @@ _io_StringIO_readable_impl(stringio *self) } /*[clinic input] +@critical_section _io.StringIO.writable Returns True if the IO object can be written. @@ -778,7 +789,7 @@ Returns True if the IO object can be written. static PyObject * _io_StringIO_writable_impl(stringio *self) -/*[clinic end generated code: output=13e4dd77187074ca input=7a691353aac38835]*/ +/*[clinic end generated code: output=13e4dd77187074ca input=1b3c63dbaa761c69]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -786,6 +797,7 @@ _io_StringIO_writable_impl(stringio *self) } /*[clinic input] +@critical_section _io.StringIO.seekable Returns True if the IO object can be seeked. @@ -793,7 +805,7 @@ Returns True if the IO object can be seeked. static PyObject * _io_StringIO_seekable_impl(stringio *self) -/*[clinic end generated code: output=4d20b4641c756879 input=4c606d05b32952e6]*/ +/*[clinic end generated code: output=4d20b4641c756879 input=a820fad2cf085fc3]*/ { CHECK_INITIALIZED(self); CHECK_CLOSED(self); @@ -812,8 +824,15 @@ _io_StringIO_seekable_impl(stringio *self) supported. */ +/*[clinic input] +@critical_section +_io.StringIO.__getstate__ + +[clinic start generated code]*/ + static PyObject * -stringio_getstate(stringio *self, PyObject *Py_UNUSED(ignored)) +_io_StringIO___getstate___impl(stringio *self) +/*[clinic end generated code: output=780be4a996410199 input=76f27255ef83bb92]*/ { PyObject *initvalue = _io_StringIO_getvalue_impl(self); PyObject *dict; @@ -839,8 +858,17 @@ stringio_getstate(stringio *self, PyObject *Py_UNUSED(ignored)) return state; } +/*[clinic input] +@critical_section +_io.StringIO.__setstate__ + + state: object + +[clinic start generated code]*/ + static PyObject * -stringio_setstate(stringio *self, PyObject *state) +_io_StringIO___setstate___impl(stringio *self, PyObject *state) +/*[clinic end generated code: output=cb3962bc6d5c5609 input=57ab906877b6a460]*/ { PyObject *initarg; PyObject *position_obj; @@ -962,7 +990,11 @@ stringio_newlines(stringio *self, void *context) CHECK_CLOSED(self); if (self->decoder == NULL) Py_RETURN_NONE; - return PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); + PyObject *result = NULL; + Py_BEGIN_CRITICAL_SECTION(self); + result = PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); + Py_END_CRITICAL_SECTION(); + return result; } #define clinic_state() (find_io_state_by_def(Py_TYPE(self))) @@ -983,8 +1015,8 @@ static struct PyMethodDef stringio_methods[] = { _IO_STRINGIO_READABLE_METHODDEF _IO_STRINGIO_WRITABLE_METHODDEF - {"__getstate__", (PyCFunction)stringio_getstate, METH_NOARGS}, - {"__setstate__", (PyCFunction)stringio_setstate, METH_O}, + _IO_STRINGIO___GETSTATE___METHODDEF + _IO_STRINGIO___SETSTATE___METHODDEF {NULL, NULL} /* sentinel */ }; From 9e183ee5f73d5f67d0b9bf68ee0dedf66ca1f369 Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 15 Nov 2023 23:35:41 +0800 Subject: [PATCH 02/12] add news entry --- .../next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst diff --git a/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst b/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst new file mode 100644 index 00000000000000..53d7ed2ec91a71 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst @@ -0,0 +1 @@ +Using critical sections to make ``io.StringIO`` thread safe. From 15e972b609c5a93aed0e2497cc38ce25f039047f Mon Sep 17 00:00:00 2001 From: AN Long Date: Thu, 16 Nov 2023 20:20:20 +0800 Subject: [PATCH 03/12] add critical sections to getters in StringIO --- Modules/_io/stringio.c | 54 +++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index cc4a56be3e7fc3..949b10ff63b849 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -971,28 +971,64 @@ _io_StringIO___setstate___impl(stringio *self, PyObject *state) static PyObject * stringio_closed(stringio *self, void *context) { - CHECK_INITIALIZED(self); - return PyBool_FromLong(self->closed); + PyObject *result = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + if (self->ok <= 0) { + PyErr_SetString(PyExc_ValueError, + "I/O operation on uninitialized object"); + goto cleanup; + }; + result = PyBool_FromLong(self->closed); + +cleanup: + Py_END_CRITICAL_SECTION(); + return result; } static PyObject * stringio_line_buffering(stringio *self, void *context) { - CHECK_INITIALIZED(self); - CHECK_CLOSED(self); - Py_RETURN_FALSE; + PyObject *result = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + if (self->ok <= 0) { + PyErr_SetString(PyExc_ValueError, + "I/O operation on uninitialized object"); + goto cleanup; + }; + if (self->closed) { + PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); + goto cleanup; + }; + result = Py_NewRef(Py_False); + +cleanup: + Py_END_CRITICAL_SECTION(); + return result; } static PyObject * stringio_newlines(stringio *self, void *context) { - CHECK_INITIALIZED(self); - CHECK_CLOSED(self); - if (self->decoder == NULL) - Py_RETURN_NONE; PyObject *result = NULL; + Py_BEGIN_CRITICAL_SECTION(self); + if (self->ok <= 0) { + PyErr_SetString(PyExc_ValueError, + "I/O operation on uninitialized object"); + goto cleanup; + }; + if (self->closed) { + PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); + goto cleanup; + }; + if (self->decoder == NULL) { + goto cleanup; + } result = PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); + +cleanup: Py_END_CRITICAL_SECTION(); return result; } From 1974630e0ee421c430b12a7c4a298b21ba02f18b Mon Sep 17 00:00:00 2001 From: AN Long Date: Thu, 16 Nov 2023 22:03:21 +0800 Subject: [PATCH 04/12] fix invalid return value in newlines --- Modules/_io/stringio.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 949b10ff63b849..95399a01215145 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -977,11 +977,11 @@ stringio_closed(stringio *self, void *context) if (self->ok <= 0) { PyErr_SetString(PyExc_ValueError, "I/O operation on uninitialized object"); - goto cleanup; + goto exit; }; result = PyBool_FromLong(self->closed); -cleanup: +exit: Py_END_CRITICAL_SECTION(); return result; } @@ -995,15 +995,15 @@ stringio_line_buffering(stringio *self, void *context) if (self->ok <= 0) { PyErr_SetString(PyExc_ValueError, "I/O operation on uninitialized object"); - goto cleanup; + goto exit; }; if (self->closed) { PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); - goto cleanup; + goto exit; }; result = Py_NewRef(Py_False); -cleanup: +exit: Py_END_CRITICAL_SECTION(); return result; } @@ -1017,18 +1017,19 @@ stringio_newlines(stringio *self, void *context) if (self->ok <= 0) { PyErr_SetString(PyExc_ValueError, "I/O operation on uninitialized object"); - goto cleanup; + goto exit; }; if (self->closed) { PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); - goto cleanup; + goto exit; }; if (self->decoder == NULL) { - goto cleanup; + result = Py_NewRef(Py_None); + goto exit; } result = PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); -cleanup: +exit: Py_END_CRITICAL_SECTION(); return result; } From 247afb0ec55d84d3ac7cbcae03425f9031a78bd3 Mon Sep 17 00:00:00 2001 From: AN Long Date: Fri, 17 Nov 2023 16:15:38 +0800 Subject: [PATCH 05/12] refactor getters of io.StringIO --- Modules/_io/stringio.c | 72 ++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 95399a01215145..8af3a630f49f0a 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -969,67 +969,57 @@ _io_StringIO___setstate___impl(stringio *self, PyObject *state) static PyObject * -stringio_closed(stringio *self, void *context) +stringio_closed_impl(stringio *self, void *context) { - PyObject *result = NULL; + CHECK_INITIALIZED(self); + return PyBool_FromLong(self->closed); +} +static PyObject * +stringio_closed(stringio *self, void *context) +{ + PyObject *result; Py_BEGIN_CRITICAL_SECTION(self); - if (self->ok <= 0) { - PyErr_SetString(PyExc_ValueError, - "I/O operation on uninitialized object"); - goto exit; - }; - result = PyBool_FromLong(self->closed); - -exit: + result = stringio_closed_impl(self, context); Py_END_CRITICAL_SECTION(); return result; } static PyObject * -stringio_line_buffering(stringio *self, void *context) +stringio_line_buffering_impl(stringio *self, void *context) { - PyObject *result = NULL; + CHECK_INITIALIZED(self); + CHECK_CLOSED(self); + Py_RETURN_FALSE; +} +static PyObject * +stringio_line_buffering(stringio *self, void *context) +{ + PyObject *result; Py_BEGIN_CRITICAL_SECTION(self); - if (self->ok <= 0) { - PyErr_SetString(PyExc_ValueError, - "I/O operation on uninitialized object"); - goto exit; - }; - if (self->closed) { - PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); - goto exit; - }; - result = Py_NewRef(Py_False); - -exit: + result = stringio_line_buffering_impl(self, context); Py_END_CRITICAL_SECTION(); return result; } static PyObject * -stringio_newlines(stringio *self, void *context) +stringio_newlines_impl(stringio *self, void *context) { - PyObject *result = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - if (self->ok <= 0) { - PyErr_SetString(PyExc_ValueError, - "I/O operation on uninitialized object"); - goto exit; - }; - if (self->closed) { - PyErr_SetString(PyExc_ValueError, "I/O operation on closed file"); - goto exit; - }; + CHECK_INITIALIZED(self); + CHECK_CLOSED(self); if (self->decoder == NULL) { - result = Py_NewRef(Py_None); - goto exit; + Py_RETURN_NONE; } - result = PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); + return PyObject_GetAttr(self->decoder, &_Py_ID(newlines)); +} -exit: +static PyObject * +stringio_newlines(stringio *self, void *context) +{ + PyObject *result; + Py_BEGIN_CRITICAL_SECTION(self); + result = stringio_newlines_impl(self, context); Py_END_CRITICAL_SECTION(); return result; } From e890485e36bf63f7e3add909972fc5df1a3e48a4 Mon Sep 17 00:00:00 2001 From: AN Long Date: Fri, 17 Nov 2023 18:53:15 +0800 Subject: [PATCH 06/12] remove news entry --- .../next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst diff --git a/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst b/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst deleted file mode 100644 index 53d7ed2ec91a71..00000000000000 --- a/Misc/NEWS.d/next/Library/2023-11-15-23-20-41.gh-issue-111965.rAPbmn.rst +++ /dev/null @@ -1 +0,0 @@ -Using critical sections to make ``io.StringIO`` thread safe. From 4f8155ef86c119187a7c481c9e33f6dc8e6a4692 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 18 Nov 2023 16:57:23 +0800 Subject: [PATCH 07/12] Update Modules/_io/stringio.c Co-authored-by: Donghee Na --- Modules/_io/stringio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 8af3a630f49f0a..788491e5653946 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -863,7 +863,7 @@ _io_StringIO___getstate___impl(stringio *self) _io.StringIO.__setstate__ state: object - + / [clinic start generated code]*/ static PyObject * From e6f50414e578f795a4d0a65e4ccd8da549d1bf66 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 18 Nov 2023 16:59:16 +0800 Subject: [PATCH 08/12] update generated files --- Modules/_io/clinic/stringio.c.h | 41 ++++----------------------------- Modules/_io/stringio.c | 2 +- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/Modules/_io/clinic/stringio.c.h b/Modules/_io/clinic/stringio.c.h index 9fd116c026e1f1..68a74ac7309ad6 100644 --- a/Modules/_io/clinic/stringio.c.h +++ b/Modules/_io/clinic/stringio.c.h @@ -452,58 +452,25 @@ _io_StringIO___getstate__(stringio *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(_io_StringIO___setstate____doc__, -"__setstate__($self, /, state)\n" +"__setstate__($self, state, /)\n" "--\n" "\n"); #define _IO_STRINGIO___SETSTATE___METHODDEF \ - {"__setstate__", _PyCFunction_CAST(_io_StringIO___setstate__), METH_FASTCALL|METH_KEYWORDS, _io_StringIO___setstate____doc__}, + {"__setstate__", (PyCFunction)_io_StringIO___setstate__, METH_O, _io_StringIO___setstate____doc__}, static PyObject * _io_StringIO___setstate___impl(stringio *self, PyObject *state); static PyObject * -_io_StringIO___setstate__(stringio *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +_io_StringIO___setstate__(stringio *self, PyObject *state) { PyObject *return_value = NULL; - #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) - - #define NUM_KEYWORDS 1 - static struct { - PyGC_Head _this_is_not_used; - PyObject_VAR_HEAD - PyObject *ob_item[NUM_KEYWORDS]; - } _kwtuple = { - .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) - .ob_item = { &_Py_ID(state), }, - }; - #undef NUM_KEYWORDS - #define KWTUPLE (&_kwtuple.ob_base.ob_base) - - #else // !Py_BUILD_CORE - # define KWTUPLE NULL - #endif // !Py_BUILD_CORE - - static const char * const _keywords[] = {"state", NULL}; - static _PyArg_Parser _parser = { - .keywords = _keywords, - .fname = "__setstate__", - .kwtuple = KWTUPLE, - }; - #undef KWTUPLE - PyObject *argsbuf[1]; - PyObject *state; - args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, argsbuf); - if (!args) { - goto exit; - } - state = args[0]; Py_BEGIN_CRITICAL_SECTION(self); return_value = _io_StringIO___setstate___impl(self, state); Py_END_CRITICAL_SECTION(); -exit: return return_value; } -/*[clinic end generated code: output=dedaeb21e1ed20c6 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=beb2b198d9c58802 input=a9049054013a1b77]*/ diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 788491e5653946..7b89e1bf136131 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -868,7 +868,7 @@ _io.StringIO.__setstate__ static PyObject * _io_StringIO___setstate___impl(stringio *self, PyObject *state) -/*[clinic end generated code: output=cb3962bc6d5c5609 input=57ab906877b6a460]*/ +/*[clinic end generated code: output=cb3962bc6d5c5609 input=8a27784b11b82e47]*/ { PyObject *initarg; PyObject *position_obj; From ac1b8e7e3c1ea6b7427d298053ae2c3ba8cb808b Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 18 Nov 2023 16:59:58 +0800 Subject: [PATCH 09/12] update include headers --- Modules/_io/stringio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 7b89e1bf136131..e907d5e9f7837e 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -1,7 +1,7 @@ #include "Python.h" -#include // offsetof() +#include // offsetof() #include "pycore_object.h" -#include "pycore_critical_section.h" +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "_iomodule.h" /* Implementation note: the buffer is always at least one character longer From 79bbff5a5fe5f62f2397185adb88c948253cafa7 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 18 Nov 2023 17:03:17 +0800 Subject: [PATCH 10/12] update global strings --- Include/internal/pycore_global_objects_fini_generated.h | 1 - Include/internal/pycore_global_strings.h | 1 - Include/internal/pycore_runtime_init_generated.h | 1 - Include/internal/pycore_unicodeobject_generated.h | 3 --- 4 files changed, 6 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index e2c248317e8822..0808076f44de31 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -1190,7 +1190,6 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(src_dir_fd)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(stacklevel)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(start)); - _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(state)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(statement)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(status)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(stderr)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 267094357feba1..8d22a9ba261010 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -679,7 +679,6 @@ struct _Py_global_strings { STRUCT_FOR_ID(src_dir_fd) STRUCT_FOR_ID(stacklevel) STRUCT_FOR_ID(start) - STRUCT_FOR_ID(state) STRUCT_FOR_ID(statement) STRUCT_FOR_ID(status) STRUCT_FOR_ID(stderr) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index bdc5e008c5790d..d41a7478db663f 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -1188,7 +1188,6 @@ extern "C" { INIT_ID(src_dir_fd), \ INIT_ID(stacklevel), \ INIT_ID(start), \ - INIT_ID(state), \ INIT_ID(statement), \ INIT_ID(status), \ INIT_ID(stderr), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index 701030c03f46e1..0c02e902b308e3 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -1878,9 +1878,6 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { string = &_Py_ID(start); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); - string = &_Py_ID(state); - assert(_PyUnicode_CheckConsistency(string, 1)); - _PyUnicode_InternInPlace(interp, &string); string = &_Py_ID(statement); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); From 0fbf608ef05f138aed8535ad54475ee7c75f7e0d Mon Sep 17 00:00:00 2001 From: AN Long Date: Sun, 19 Nov 2023 15:01:41 +0800 Subject: [PATCH 11/12] update clinic generate files --- Modules/_io/clinic/stringio.c.h | 3 ++- Modules/_io/stringio.c | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_io/clinic/stringio.c.h b/Modules/_io/clinic/stringio.c.h index 68a74ac7309ad6..8e5c687dc6a55f 100644 --- a/Modules/_io/clinic/stringio.c.h +++ b/Modules/_io/clinic/stringio.c.h @@ -7,6 +7,7 @@ preserve # include "pycore_runtime.h" // _Py_ID() #endif #include "pycore_abstract.h" // _Py_convert_optional_to_ssize_t() +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_CheckPositional() PyDoc_STRVAR(_io_StringIO_getvalue__doc__, @@ -473,4 +474,4 @@ _io_StringIO___setstate__(stringio *self, PyObject *state) return return_value; } -/*[clinic end generated code: output=beb2b198d9c58802 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=5c8d67f4408a1e6e input=a9049054013a1b77]*/ diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index e907d5e9f7837e..79b8c1903da127 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -1,7 +1,6 @@ #include "Python.h" -#include // offsetof() +#include // offsetof() #include "pycore_object.h" -#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "_iomodule.h" /* Implementation note: the buffer is always at least one character longer From 0e23c9c7428823da0b32eb113f66b158828b1109 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sun, 19 Nov 2023 15:25:35 +0800 Subject: [PATCH 12/12] adjust clinic header include location --- Modules/_io/stringio.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_io/stringio.c b/Modules/_io/stringio.c index 79b8c1903da127..0aa5e34cd7c8b2 100644 --- a/Modules/_io/stringio.c +++ b/Modules/_io/stringio.c @@ -45,6 +45,10 @@ typedef struct { _PyIO_State *module_state; } stringio; +#define clinic_state() (find_io_state_by_def(Py_TYPE(self))) +#include "clinic/stringio.c.h" +#undef clinic_state + static int _io_StringIO___init__(PyObject *self, PyObject *args, PyObject *kwargs); #define CHECK_INITIALIZED(self) \ @@ -1023,10 +1027,6 @@ stringio_newlines(stringio *self, void *context) return result; } -#define clinic_state() (find_io_state_by_def(Py_TYPE(self))) -#include "clinic/stringio.c.h" -#undef clinic_state - static struct PyMethodDef stringio_methods[] = { _IO_STRINGIO_CLOSE_METHODDEF _IO_STRINGIO_GETVALUE_METHODDEF