Skip to content

Fix poll pending#5

Open
quettabit wants to merge 1 commit intomechiru:masterfrom
quettabit:waker-fix
Open

Fix poll pending#5
quettabit wants to merge 1 commit intomechiru:masterfrom
quettabit:waker-fix

Conversation

@quettabit
Copy link

Copy link

@shikhar shikhar left a comment

Choose a reason for hiding this comment

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

Makes sense!

Copy link

@shikhar shikhar left a comment

Choose a reason for hiding this comment

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

inline comment

},
Poll::Pending => break Poll::Pending,
Poll::Pending => {
cx.waker().wake_by_ref();
Copy link

Choose a reason for hiding this comment

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

Just realized, I think only the above wake is necessary. In this case the inner future being polled is responsible for waking.

Copy link
Author

@quettabit quettabit Mar 12, 2022

Choose a reason for hiding this comment

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

valid point. i assumed the inner future was not doing its job and just added one here. this fixes the issue, but i don't have a valid reasoning about the fix itself though. also, there is a possibility that the issue could be stemming from somewhere else in the library.

the issue -- without this wake, when application-level tasks attempt to fetch the token concurrently, the tasks get hanged after fetching the token and doesn't progress further.

Copy link

Choose a reason for hiding this comment

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

Can you try to create a minimal repro?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants