gh-146065: Fix NULL dereference in FutureIter_am_send

[VanshAgarwal24036]

After throw() or close(), it->future can be NULL. A subsequent send() call would dereference it and crash. This adds a NULL check and raises StopIteration instead.

• Issue: NULL pointer dereference in FutureIter_am_send #146065

[bedevere-app]

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.

[VanshAgarwal24036]

I have made the requested changes; please review again

[bedevere-app]

Thanks for making the requested changes!

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

[JArmandoAnaya]

I was able to reproduce and verify this fix locally on macOS (x86_64).

Without the fix (NULL check in FutureIter_am_send removed): test_futureiter_send_after_throw_no_crash crashes with a segfault, confirmed NULL dereference in FutureIter_am_send when it->future is NULL after a prior throw().

With the fix (NULL check restored): all three variants of the test pass (CFutureTests, CSubFutureTests, PyFutureTests), and the full test.test_asyncio.test_futures suite passes (186/186 tests OK, no regressions).

[vstinner]

LGTM.

I confirm that test_futureiter_send_after_throw_no_crash() does crash without the fix, and pass with the fix.

@kumaraditya303: Do you want to double check this fix? I didn't work on ayncio recently, and I'm not 100% sure if raising StopIteration is the correct behavior in this case.

[VanshAgarwal24036]

This failure appears unrelated to the asyncio change. It occurs in test_ssl and looks like an environment-specific difference in SSL error messages (AWS-LC vs OpenSSL), not connected to FutureIter_am_send.

[kumaraditya303]

Do you want to double check this fix? I didn't work on ayncio recently, and I'm not 100% sure if raising StopIteration is the correct behavior in this case.

I'll take a closer look later but reading it->future outside the critical section looks unsafe.

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[orsenthil]

We need to have an additional test case for the it.close() similar to this.

[orsenthil]

but reading it->future outside the critical section looks unsafe.

This was the previous reviewer's @kumaraditya303 review comment.

There is also a read happening in FutureIter_am_send_lock_held (line 1832) which is called from FutureIter_am_send

After reading it->future in a critical section, sending the same object might address it.

@VanshAgarwal24036, I noticed the PR has become state. Would you like to revive it again with these change ?

[msullivan]

Do you want to double check this fix? I didn't work on ayncio recently, and I'm not 100% sure if raising StopIteration is the correct behavior in this case.

I'll take a closer look later but reading it->future outside the critical section looks unsafe.

Yeah, I think you are right about this. I put up #158468 as a version that moves all the accesses into critical sections.