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.

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

untrack_task() is_some check occasionally fails #1756

Closed
kevinkassimo opened this issue Feb 13, 2019 · 0 comments
Closed

untrack_task() is_some check occasionally fails #1756

kevinkassimo opened this issue Feb 13, 2019 · 0 comments

Comments

@kevinkassimo
Copy link
Contributor

When running two wrk benchmarks one immediately after another, I almost always get

thread 'tokio-runtime-worker-0' panicked at 'assertion failed: t.is_some()', ../../src/resources.rs:205:7
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "PoisonError { inner: .. }"', src/libcore/result.rs:997:5
fatal runtime error: failed to initiate panic, error 5
Abort trap: 6

Basically,

deno/src/resources.rs

Lines 199 to 208 in b2fb826

/// Stop tracking a task (for TcpListener resource).
/// Happens when the task is done and thus no further tracking is needed.
pub fn untrack_task(&mut self) {
let mut table = RESOURCE_TABLE.lock().unwrap();
// Only untrack if is TcpListener.
if let Some(Repr::TcpListener(_, t)) = table.get_mut(&self.rid) {
assert!(t.is_some());
t.take();
}
}

assert!(t.is_some()); fails.

From my investigation,

deno/src/tokio_util.rs

Lines 64 to 86 in b2fb826

fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
let (stream, addr) = match self.state {
// Similar to try_ready!, but also track/untrack accept task
// in TcpListener resource.
// In this way, when the listener is closed, the task can be
// notified to error out (instead of stuck forever).
AcceptState::Pending(ref mut r) => match r.poll_accept() {
Ok(futures::prelude::Async::Ready(t)) => {
r.untrack_task();
t
}
Ok(futures::prelude::Async::NotReady) => {
// Would error out if another accept task is being tracked.
r.track_task()?;
return Ok(futures::prelude::Async::NotReady);
}
Err(e) => {
r.untrack_task();
return Err(e);
}
},
AcceptState::Empty => panic!("poll Accept after it's done"),
};

some connections immediately yields Ok(futures::prelude::Async::Ready(t)) without ever yields Ok(futures::prelude::Async::NotReady). Thus no task is waiting => assertion would fail.

This happens because in eager_unix tcp_accept,

deno/src/eager_unix.rs

Lines 68 to 84 in b2fb826

match result {
Ok((std_stream, addr)) => {
let result = tokio::net::TcpStream::from_std(
std_stream,
&tokio::reactor::Handle::default(),
);
let tokio_stream = result.unwrap();
Either::B(future::ok((tokio_stream, addr)))
}
Err(err) => {
if err.kind() == ErrorKind::WouldBlock {
Either::A(tokio_util::accept(resource))
} else {
Either::B(future::err(err))
}
}
}

It is possible that an accept attempt would yield WouldBlock on first check, but would immediately success on the first poll once spawned in thread pool as a Future.

To address this issue, we need to remove the assertion from untrack_task()

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

No branches or pull requests

1 participant