Skip to content

Conversation

@Yashp002
Copy link
Contributor

@Yashp002 Yashp002 commented Nov 16, 2025

This PR fixes an incomplete code example in the concurrent.futures documentation.

The second example in the deadlock section was missing a .result() call on the executor.submit() invocation, which is necessary to demonstrate the deadlock behavior described in the comment.

Changes:

  • Added .result() call to complete the example in Doc/library/concurrent.futures.rst

Fixes #141617


📚 Documentation preview 📚: https://cpython-previews--141620.org.readthedocs.build/

@picnixz picnixz self-requested a review November 30, 2025 19:40
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

The other place where to add the comment was after b = executor.submit(wait_on_a) just a few lines above not in the documentation for submit.

Comment on lines 43 to 45
# Note: calling future.result() outside this with statement would work fine,
# but calling it here is safe because we print it immediately and the executor
# can complete the task before the context manager exits.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is incorrect. It's not for this example that we needed to add this comment.

executor = ThreadPoolExecutor(max_workers=1)
executor.submit(wait_on_future)

executor.submit(wait_on_future).result()
Copy link
Member

Choose a reason for hiding this comment

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

We should not call .result(). We should indicate in the comment that calling .result would produce a deadlock.

@bedevere-app
Copy link

bedevere-app bot commented Nov 30, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@Yashp002 Yashp002 requested a review from picnixz December 2, 2025 18:46
@Yashp002
Copy link
Contributor Author

Yashp002 commented Dec 2, 2025

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Dec 2, 2025

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@Yashp002
Copy link
Contributor Author

Yashp002 commented Dec 9, 2025

Hi @picnixz Just checking in on this PR. I'm happy to address any remaining feedback or make changes as needed. Please let me know if there's anything I should update. Thanks.

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

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Incomplete example in concurrent.future documentation

2 participants