Skip to content

gh-111965: Use critical sections to make io.TextIOWrapper thread safe - #112193

Merged
corona10 merged 7 commits into
python:mainfrom
aisk:critical-sections-textiowrapper
Nov 18, 2023
Merged

gh-111965: Use critical sections to make io.TextIOWrapper thread safe#112193
corona10 merged 7 commits into
python:mainfrom
aisk:critical-sections-textiowrapper

Conversation

@aisk

@aiskaisk commented Nov 17, 2023•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@aiskaisk changed the title ssing critical sections to make io.TextIOWrapper thread safegh-111965: using critical sections to make io.TextIOWrapper thread safeNov 17, 2023
@bedevere-appbedevere-appBot mentioned this pull request Nov 17, 2023
Closed
corona10
corona10 reviewed Nov 17, 2023
View reviewed changes
Comment threadMisc/NEWS.d/next/Library/2023-11-17-17-34-30.gh-issue-111965.6iMWn0.rst Outdated
@@ -0,0 +1 @@
Using critical section to make ``io.TextIOWrapper`` thread safe without GIL.

@corona10corona10Nov 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As I commented before, NEWS.d is not needed for this task

@aiskaiskNov 17, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Sorry, just noticed it

@colesbury
colesbury self-requested a review November 17, 2023 14:34
@colesburycolesbury changed the title gh-111965: using critical sections to make io.TextIOWrapper thread safegh-111965: Use critical sections to make io.TextIOWrapper thread safeNov 17, 2023
colesbury
colesbury approved these changes Nov 17, 2023
View reviewed changes

@colesburycolesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@corona10 would you please look over this one as well?

corona10
corona10 requested changes Nov 18, 2023
View reviewed changes
Comment threadModules/_io/textio.c Outdated
#include "Python.h"
#include "pycore_call.h" // _PyObject_CallMethod()
#include "pycore_codecs.h" // _PyCodecInfo_GetIncrementalDecoder()
#include "pycore_critical_section.h"

@corona10corona10Nov 18, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a comment as the same as the other header. // Py_BEGIN_CRITICAL_SECTION()

@aiskaiskNov 18, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Updated

corona10
corona10 reviewed Nov 18, 2023
View reviewed changes
Comment threadModules/_io/textio.c Outdated
@corona10corona10 mentioned this pull request Nov 18, 2023
Merged
aiskand others added 2 commits November 18, 2023 16:31
@aisk@corona10
Update Modules/_io/textio.c
1898faa
Co-authored-by: Donghee Na <donghee.na@python.org>
@aisk
align include headers
da2d2c1
corona10
corona10 approved these changes Nov 18, 2023
View reviewed changes

@corona10corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@corona10
corona10 merged commit 1a969b4 into python:mainNov 18, 2023
@aisk

aisk commented Nov 19, 2023

Copy link
Copy Markdown
MemberAuthor

Thanks for the review!

@aisk
aisk deleted the critical-sections-textiowrapper branch November 19, 2023 06:56
aisk added a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@aisk
pythongh-111965: Use critical sections to make io.TextIOWrapper threa…
7146788
…d safe (pythongh-112193)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@aisk@Glyphack
pythongh-111965: Use critical sections to make io.TextIOWrapper threa…
c937228
…d safe (pythongh-112193)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesburycolesburycolesbury approved these changes
@corona10corona10corona10 approved these changes

Assignees

No one assigned

Labels

skip newstopic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aisk@colesbury@corona10