Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(client): enter tokio runtime context before calling timeout in recv_timeout #492

Merged
merged 2 commits into from Nov 1, 2022
Merged

fix(client): enter tokio runtime context before calling timeout in recv_timeout #492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3414827
fix: recv_timeout
swanandx Oct 29, 2022
62d0ea4
updated changes in changelog
swanandx Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
- Removed unused dependencies and updated version of some of used libraries to fix dependabots warning (#475)
- (rumqttd) Added properties field to `Unsubscribe`, `UnsubAck`, and `Disconnect` packets so its consistent with other packets. (#480)
- (rumqttd) Changed default segment size in demo config to 100MB (#484)
- (rumqttc) Fixed panicking of `timeout` in `recv_timeout` by entering tokio runtime context (#492)

### R17
---
Expand Down
3 changes: 3 additions & 0 deletions rumqttc/src/client.rs
View file
Open in desktop
Expand Up @@ -427,6 +427,9 @@ impl Connection {
&mut self,
duration: Duration,
) -> Result<Result<Event, ConnectionError>, RecvTimeoutError> {
// Enter the runtime so we can use Sleep, which is required by timeout.
// ref: https://docs.rs/tokio/latest/tokio/runtime/struct.Runtime.html#method.enter
let _guard = self.runtime.enter();
Copy link
Member

@de-sh de-sh Nov 9, 2022 •
edited

Choose a reason for hiding this comment

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

Seem like this could have been fixed by ensuring the construction of the timeout happened within an async block passed to block_on without having to explicitly enter the runtime context. Have tested and verified its working.

.block_on(async { timeout(duration, f).await })

Copy link
Member Author

Choose a reason for hiding this comment

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

I did try it out, but for me it didn't work. That is why I ended up choosing the solution of manually entering runtime.

Copy link
Member

Choose a reason for hiding this comment

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

I believe since the timeout would be created outside the runtime context before being passed into block_on(), the reactor would be deemed missing whereas within async { } the timeout is only created once the external future gets polled from within the runtime.

Thus why .block_on(timeout(duration, f)) behaves different to .block_on(async { timeout(duration, f).await }).

Copy link
Member Author

@swanandx swanandx Nov 10, 2022 •
edited

Choose a reason for hiding this comment

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

Yeah you are right, I was creating timeout future first and then passing it down in that async block inside block_on 😅. Good catch! If entering guard isn't required, can we remove it in your PR?

let f = timeout(duration, self.eventloop.poll());
let event = self
.runtime
Expand Down
3 changes: 3 additions & 0 deletions rumqttc/src/v5/client.rs
View file
Open in desktop
Expand Up @@ -430,6 +430,9 @@ impl Connection {
&mut self,
duration: Duration,
) -> Result<Result<Event, ConnectionError>, RecvTimeoutError> {
// Enter the runtime so we can use Sleep, which is required by timeout.
// ref: https://docs.rs/tokio/latest/tokio/runtime/struct.Runtime.html#method.enter
let _guard = self.runtime.enter();
let f = timeout(duration, self.eventloop.poll());
let event = self
.runtime
Expand Down