Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jun 14, 2019

Join the thread to prevent leaking a running thread and leaking a
reference.

Cleanup also the test:

  • asyncioWindowsProactorEventLoopPolicy became the default policy,
    there is no need to set it manually.
  • Only start the thread once the loop is running.
  • Use a shorter sleep in the thread (100 ms rather than 1 sec).
  • Use close_loop(loop) rather than loop.close().
  • Use longer variable names.

https://bugs.python.org/issue37278

@vstinner vstinner requested a review from asvetlov June 14, 2019 10:21
@vstinner vstinner requested a review from 1st1 as a code owner June 14, 2019 10:21
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting core review labels Jun 14, 2019
Join the thread to prevent leaking a running thread and leaking a
reference.

Cleanup also the test:

* asyncioWindowsProactorEventLoopPolicy became the default policy,
  there is no need to set it manually.
* Only start the thread once the loop is running.
* Use a shorter sleep in the thread (100 ms rather than 1 sec).
* Use close_loop(loop) rather than loop.close().
* Use longer variable names.
Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks!

@vstinner
Copy link
Member Author

I tested manually on Windows that the test no longer leaks:

vstinner@WIN C:\vstinner\python\master>python -m test -R 3:3 test_asyncio -m test.test_asyncio.test_windows_events.ProactorLoopCtrlC.test_ctrl_c
Running Debug|x64 interpreter...
Run tests sequentially
0:00:00 load avg: 0.00 [1/1] test_asyncio
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

1 test OK.

Total duration: 8 sec 31 ms
Tests result: SUCCESS

@vstinner vstinner merged commit 0755945 into python:master Jun 14, 2019
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-14077 is a backport of this pull request to the 3.8 branch.

@vstinner vstinner deleted the test_asyncio_ctrl_c branch June 14, 2019 11:04
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-14078 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Jun 14, 2019
Join the thread to prevent leaking a running thread and leaking a
reference.

Cleanup also the test:

* asyncioWindowsProactorEventLoopPolicy became the default policy,
  there is no need to set it manually.
* Only start the thread once the loop is running.
* Use a shorter sleep in the thread (100 ms rather than 1 sec).
* Use close_loop(loop) rather than loop.close().
* Use longer variable names.
(cherry picked from commit 0755945)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
Join the thread to prevent leaking a running thread and leaking a
reference.

Cleanup also the test:

* asyncioWindowsProactorEventLoopPolicy became the default policy,
  there is no need to set it manually.
* Only start the thread once the loop is running.
* Use a shorter sleep in the thread (100 ms rather than 1 sec).
* Use close_loop(loop) rather than loop.close().
* Use longer variable names.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Join the thread to prevent leaking a running thread and leaking a
reference.

Cleanup also the test:

* asyncioWindowsProactorEventLoopPolicy became the default policy,
  there is no need to set it manually.
* Only start the thread once the loop is running.
* Use a shorter sleep in the thread (100 ms rather than 1 sec).
* Use close_loop(loop) rather than loop.close().
* Use longer variable names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants