fix(client): enter tokio runtime context before calling timeout in recv_timeout #492
Conversation
| @@ -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(); | |||
There was a problem hiding this comment.
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 })
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 }).
There was a problem hiding this comment.
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?
…cv_timeout (bytebeamio#492) * fix: recv_timeout * updated changes in changelog
timeoutfunction usesSleepinternally, but as we are outside of Tokio runtime context, it will fail to spawn it and thus will panic! reference.To fix it, we can explicitly enter the runtime by using enter method.
Fixes Issue: #467