Skip to content

Conversation

@yihong0618
Copy link
Contributor

@yihong0618 yihong0618 commented Oct 29, 2025

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618 yihong0618 changed the title fix: Add __mp_main__ as a duplicate for __main__ for pickle to work gh-140729: Add __mp_main__ as a duplicate for __main__ for pickle to work Oct 29, 2025
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
Contributor Author

test failed is flaky

@@ -0,0 +1,2 @@
Fix: Add __mp_main__ as a duplicate for __main__ for pickle to work in
Copy link
Member

Choose a reason for hiding this comment

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

This needs to refer the profiler otherwise is not possible to undestand what this is fixing. Also this should say WHAT is fixed not HOW

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618 yihong0618 requested a review from pablogsal November 1, 2025 22:43
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
Contributor Author

Do not know why windows free thread failed root cause, will go to find a windows test it these days.

@pablogsal
Copy link
Member

Do not know why windows free thread failed root cause, will go to find a windows test it these days.

It seems there is some kind of race condition because the other windows test failed with a timeout

with SuppressCrashReport():
with script_helper.spawn_python(
"-m", "profiling.sampling.sample",
"-d", "1",
Copy link
Member

Choose a reason for hiding this comment

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

You probaby need to sample for more time in case the machine is slow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check other code in the test use 5 now

stderr=subprocess.PIPE,
text=True
) as proc:
proc.wait(timeout=10)
Copy link
Member

@pablogsal pablogsal Nov 2, 2025

Choose a reason for hiding this comment

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

Use SHORT_TIMEOUT from support (you are already importing it)

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
Contributor Author

Do not know why windows free thread failed root cause, will go to find a windows test it these days.

It seems there is some kind of race condition because the other windows test failed with a timeout

passed now, thank you very much, learned that

@yihong0618 yihong0618 requested a review from pablogsal November 2, 2025 01:54
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 3, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 59c1c1d 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140735%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 3, 2025
@YvesDup
Copy link
Contributor

YvesDup commented Nov 4, 2025

Regarding our recent discussions, could you add the fix about the cProfile module and a test ?

@yihong0618
Copy link
Contributor Author

Regarding our recent discussions, could you add the fix about the cProfiler module and a test ?

sorry for the comment, I think we should not fix it here

I we should not fix this issue on the profile module indeed

@yihong0618
Copy link
Contributor Author

conflict fixed

@pablogsal
Copy link
Member

Amazing! Thanks a lot for the great work @yihong0618 🚀

@pablogsal pablogsal merged commit 994ab5c into python:main Nov 17, 2025
50 checks passed
@yihong0618
Copy link
Contributor Author

Amazing! Thanks a lot for the great work @yihong0618 🚀

thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants