reset woken of outer future after polled #4157
Conversation
Darksonn
added
A-tokio
|
It seems not easy to test. If I want to test this, I have to change many fields or structs' visibility. :) |
|
The other PR had a test that a certain future was not polled more than three times. It seems like a test like that could work here without changing the visibility of anything. |
What I want to test is |
|
You could test that checking whether it gets polled when some other spawned future gets polled, no? |
suikammd
force-pushed
the
master
branch
10 times, most recently
from
78ff17f
to
8f46f0e
Compare
cc @Darksonn @zaharidichev please review the new added test |
| // expect ResetFuture.pending_cnt = 1 | ||
| // 2. recv message from channel, ResetFuture returns Ready immediately. | ||
| // We expect ResetFuture.pending_cnt = 0 | ||
| future::ready(()).await; |
There was a problem hiding this comment.
I'm guessing you want tokio::task::yield_now here? Awaiting a future::ready doesn't actually yield.
| @@ -31,6 +31,40 @@ fn assert_at_most_num_polls(rt: Arc<Runtime>, at_most_polls: usize) { | |||
| assert!(polls <= at_most_polls); | |||
| } | |||
|
|
|||
| fn assert_no_unnecessary_poll(rt: Arc<Runtime>) { | |||
There was a problem hiding this comment.
You don't need to wrap the runtime in an Arc as its used here.
|
Please don't force push the PR when you make changes. It's a lot harder to keep track of what changed since I last looked when you do that. |
I see, I was worried about too many commits. |
|
We squash your PR on merge, so that's not a concern. |
Motivation
Current implementation does not reset woken flag after polling outer future. This results in outer future being polled again and again which is unnecessary when outer future not ready.
Solution
Swap woken flag to false at the start of loop.