Skip to content

Temporarily allow the TSan (free-threading) job to fail#152173

Merged
kumaraditya303 merged 4 commits into
python:mainfrom
StanFromIreland:test_abstractmethod_integration
Jul 7, 2026
Merged

Temporarily allow the TSan (free-threading) job to fail#152173
kumaraditya303 merged 4 commits into
python:mainfrom
StanFromIreland:test_abstractmethod_integration

Conversation

@StanFromIreland

@StanFromIreland StanFromIreland commented Jun 25, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Jun 25, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jun 25, 2026
Closed
@StanFromIreland StanFromIreland requested a review from vstinner June 25, 2026 09:13
@StanFromIreland StanFromIreland mentioned this pull request Jun 25, 2026
Merged
@StanFromIreland

StanFromIreland commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Oh, another race-condition in test_ssl. I think we can mark this job as optional instead.

@StanFromIreland StanFromIreland changed the title gh-151593: Skip test_abc.test_abstractmethod_integration on free-threading gh-151593: Temporarily allow the TSan (free-threading) job to fail Jun 25, 2026
@StanFromIreland StanFromIreland added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 25, 2026
@StanFromIreland

StanFromIreland commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Also blocking various 3.15 backports, e.g. https://github.com/python/cpython/actions/runs/28159079945/job/83394836968?pr=152169.

@vstinner

vstinner commented Jun 25, 2026

Copy link
Copy Markdown
Member

The issue is quite serious, it's a real deadlock in the dict implementation! If possible, I would prefer to not skip the test (to not forget about it), but fix the deadlock instead. But I totally agree that this bug is really annoying (it makes the CI quite often).

I created #152200 to fix the deadlock.

@StanFromIreland

StanFromIreland commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

I would prefer to not skip the test (to not forget about it)

I don't think we'd forget, it would still run and fail, leaving it at the top of the list:

image

Marking this as draft considering there's a patch, but if that gets delayed I'll reconsider.

@StanFromIreland StanFromIreland marked this pull request as draft June 25, 2026 13:37
@vstinner

vstinner commented Jun 25, 2026

Copy link
Copy Markdown
Member

I merged a fix instead: #152200.

@vstinner vstinner closed this Jun 25, 2026
@StanFromIreland StanFromIreland deleted the test_abstractmethod_integration branch June 25, 2026 19:29
@StanFromIreland StanFromIreland restored the test_abstractmethod_integration branch July 7, 2026 14:10
@StanFromIreland StanFromIreland changed the title gh-151593: Temporarily allow the TSan (free-threading) job to fail Temporarily allow the TSan (free-threading) job to fail Jul 7, 2026
@StanFromIreland StanFromIreland marked this pull request as ready for review July 7, 2026 14:11
@vstinner

vstinner commented Jul 7, 2026

Copy link
Copy Markdown
Member

I support this chance since there are now at least two known failures in "Sanitizers / TSan (free-threading)" job:

webknjaz
webknjaz reviewed Jul 7, 2026
View reviewed changes
Comment thread .github/workflows/build.yml
webknjaz
webknjaz reviewed Jul 7, 2026
View reviewed changes
Comment thread .github/workflows/build.yml
webknjaz
webknjaz reviewed Jul 7, 2026
View reviewed changes
Comment thread .github/workflows/build.yml
webknjaz
webknjaz approved these changes Jul 7, 2026
View reviewed changes

@webknjaz webknjaz 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.

Other than the possible comment improvement, LGTM.

vstinner
vstinner reviewed Jul 7, 2026
View reviewed changes

@vstinner vstinner 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.

I don't understand well GitHub Action YAML format, but the change looks good to me.

@StanFromIreland

StanFromIreland commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

CIFuzz broken because of array.ArrayType removal (will be fixed by #153261).

@vstinner

vstinner commented Jul 7, 2026

Copy link
Copy Markdown
Member

Aha, "TSan (free-threading)" CI job failed but it's possible to merge the PR. So the change works as expected (accept "TSan (free-threading)" failure).

Screenshot 2026-07-07 at 16-55-22 Temporarily allow the TSan (free-threading) job to fail by StanFromIreland · Pull Request #152173 · python_cpython

@kumaraditya303 kumaraditya303 merged commit 4f39efc into python:main Jul 7, 2026
54 of 56 checks passed
@miss-islington-app

miss-islington-app Bot commented Jul 7, 2026

Copy link
Copy Markdown

Thanks @StanFromIreland for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Jul 7, 2026

Copy link
Copy Markdown

GH-153268 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 7, 2026
@StanFromIreland StanFromIreland deleted the test_abstractmethod_integration branch July 7, 2026 15:04
StanFromIreland added a commit that referenced this pull request Jul 7, 2026
@miss-islington @StanFromIreland
[3.15] Temporarily allow the TSan (free-threading) job to fail (GH-15…
07d8f50 
…2173) (#153268)

(cherry picked from commit 4f39efc)

Co-authored-by: Stan Ulbrych <stan@python.org>
kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request Jul 8, 2026
@kumaraditya303
Revert "Temporarily allow the TSan (free-threading) job to fail (pyth…
c25b785 
…on#152173)"

This reverts commit 4f39efc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@webknjaz webknjaz webknjaz approved these changes
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner
@hugovk hugovk Awaiting requested review from hugovk hugovk is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@itamaro itamaro Awaiting requested review from itamaro itamaro is a code owner
@JacobCoffee JacobCoffee Awaiting requested review from JacobCoffee JacobCoffee is a code owner

Assignees

No one assigned

Labels

skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@StanFromIreland @vstinner @webknjaz @kumaraditya303