GeezerVison

The following commit authors need to sign the Contributor License Agreement:

• [email protected]

Click the button to sign:

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Result: ACCESS

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

I believe that the issue with my patch was due to closing the file accepting output before calling executor.shutdown(wait=True). The modified test now passes with my change.

Edit: I also feel like there's probably a cleaner way to handle pending_bytes_count but given that I'm not even certain that this is the right way to solve the problem, I'm not investing too much thought into it.

After deploying this patch at scale internally, I can confirm that there are still race condition issues; the test fails about 14% of the time in our CI cluster (interestingly it always passed while running locally, so it might be a load issue; I haven't investigated the exact nature of the failures yet). 14% is better than the 100% failure rate we were seeing prior, but it's still not fully fixed.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

I think that the test provided here might be useful (after being adapted to meet cpython standards), but the actual code change likely is not. On #119507 a much simpler patch achieves the same practical "it doesn't assert any longer" effect for my use case.

The issue is also being discussed here: https://discuss.python.org/t/bug-with-multi-threaded-print-write-causing-dropped-output-output-of-uninitialized-memory-assertion-failures/54538 where @pitrou suggests using a mutex to protect the internal state.

Should we close this PR?

@elistevens, thanks for taking the time you did on this PR. While there wasn't much feedback here, that DPO thread definitely served as a proxy, which is probably okay. Regardless, you clearly put a lot of thought into this and demonstrated a healthy dose of determination and skill. I hope there will be more opportunities for you to participate in Python core development. If you have any feedback, please let us know.

Thanks, Eric! I'm not worried about the relative quiet on this PR; #119507 had a lively discussion and ultimately resulted in a much cleaner fix than the one I proposed here. I also recognize that maintainers are quite busy and I think should focus on the PRs that are likely to merge.

Thank you for all the work that you all do!