Skip to content

test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy(), move loop creation#34820

Open
maflcko wants to merge 2 commits intobitcoin:masterfrom
maflcko:2603-test-windows-revert
Open

test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy(), move loop creation#34820
maflcko wants to merge 2 commits intobitcoin:masterfrom
maflcko:2603-test-windows-revert

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Mar 13, 2026

It is deprecated according to https://docs.python.org/3.14/library/asyncio-policy.html#asyncio.WindowsSelectorEventLoopPolicy The replacement exists since python 3.7

Also, move the event loop creation to happen in the thread that runs the loop.

@DrahtBot DrahtBot changed the title test: Use asyncio.SelectorEventLoop() over deprecated asyncio.SelectorEventLoop() test: Use asyncio.SelectorEventLoop() over deprecated asyncio.SelectorEventLoop() Mar 13, 2026
@DrahtBot DrahtBot added the Tests label Mar 13, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 13, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK l0rinc

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@maflcko maflcko changed the title test: Use asyncio.SelectorEventLoop() over deprecated asyncio.SelectorEventLoop() test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy() Mar 13, 2026
@maflcko maflcko force-pushed the 2603-test-windows-revert branch from 2222623 to fa9168f Compare March 13, 2026 11:12
if platform.system() == 'Windows':
asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
NetworkThread.network_event_loop = asyncio.new_event_loop()
NetworkThread.network_event_loop = asyncio.SelectorEventLoop() if platform.system() == "Windows" else asyncio.new_event_loop()
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the deprecation seems like a good start, but my understanding is that __init__ happens before thread.start(), and run() after it, so the event loop would still be created on the main thread and then run on the worker thread. My understanding is that this potential cross-thread handoff could still be the suspected source of flakiness.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, ok, I'll move it as well.

This should fix bitcoin#34367

I am not familiar with Windows sockets thread-safety, but creating the
event loop on the main thread, and running it in the network thread
could lead to a fast abort in Python on Windows (without any stderr):

```
77/276 - wallet_txn_clone.py failed, Duration: 1 s

stdout:
2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117

stderr:

Combine the logs and print the last 99999999 lines ...
============
Combined log for D:\a\_temp/test_runner_₿_🏃_20251210_075632/wallet_txn_clone_196:
============
 test  2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117
 test  2025-12-10T08:04:27.500433Z TestFramework (DEBUG): Setting up network thread
```

Also, I couldn't find any docs that require the loop must be created on
the thread that runs them:

* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.new_event_loop
* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_forever

However, the patch seems trivial to review, harmless, and easy to
revert, so it may be a good try to fix the intermittent Windows Python
crash.
@maflcko maflcko changed the title test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy() test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy(), move loop creation Mar 13, 2026
@l0rinc
Copy link
Contributor

l0rinc commented Mar 13, 2026

ACK fa050da

The change seems low-risk and makes sense based on my limited understanding of the situation.
Checked the related tests and they're passing on Windows and Mac.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants