compio-rs / compio

A thread-per-core Rust runtime with IOCP/io_uring/polling.
MIT License
420 stars 37 forks source link

fix(tls): avoid MISSING_BUF panic on stream shutdown; +bonus debug feature #234

Closed valpackett closed 7 months ago

valpackett commented 7 months ago

When trying to get a basic HTTPS server to work:

the code ```rust mod rustls_config; use std::{convert::Infallible, path::Path, sync::Arc}; use compio::{net::TcpListener, tls::TlsAcceptor, driver::{IntoRawFd, FromRawFd}}; use http_body_util::Full; use hyper::{ body::{Bytes, Incoming}, Request, Response, }; async fn hello(_: Request) -> Result>, Infallible> { Ok(Response::new(Full::new(Bytes::from("Hello, World!")))) } fn main() -> anyhow::Result<()> { compio::rustls::crypto::ring::default_provider() .install_default() .unwrap(); let tls_conf = rustls_config::create_rustls_config( Path::new("./xxx.crt"), Path::new("./xxx.key"), )?; let tls_acceptor: TlsAcceptor = Arc::new(tls_conf).into(); // let count = std::thread::available_parallelism()?.get(); eprintln!("Starting http server on port 8000"); let rt = compio::runtime::RuntimeBuilder::new().build().unwrap(); // let (shutdown_tx, mut shutdown_rx) = oneshot::channel(); let srv = async move { let mut sock = socket2::Socket::new(socket2::Domain::IPV6, socket2::Type::STREAM, None).unwrap(); sock.set_only_v6(false).unwrap(); sock.set_reuse_port(true).unwrap(); let address: std::net::SocketAddr = "[::0]:8000".parse().unwrap(); let address = address.into(); sock.bind(&address).unwrap(); sock.listen(128).unwrap(); let listener = unsafe { compio::net::TcpListener::from_raw_fd(sock.into_raw_fd()) }; let addr = listener.local_addr().unwrap(); let builder = hyper_util::server::conn::auto::Builder::new(cyper_core::CompioExecutor); // while let Ok(None) = shutdown_rx.try_recv() { loop { let (stream, _) = listener.accept().await.unwrap(); let stream = tls_acceptor.accept(stream).await.unwrap(); if let Err(x) = builder .serve_connection( cyper_core::HyperStream::new(stream), hyper::service::service_fn({ // let func = hello.clone(); move |req| { let fut = hello(req); async move { fut.await } } }), ) .await { eprintln!("{:?}", x); } } }; rt.block_on(srv); Ok(()) } ```

a MISSING_BUF panic would happen. I've added a debugging feature to print the backtrace of the first Buffer::take_inner if a second one occurs before the buffer is returned, and got this:

thread 'main' panicked at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:205:17:
The buffer was submitted for io and never returned

First taken at:
   0: compio_io::buffer::Buffer::take_inner
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:207:36
   1: compio_io::buffer::Buffer::with::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:155:40
   2: compio_io::buffer::Buffer::flush_to::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:179:18
   3: compio_io::compat::SyncStream<S>::flush_write_buf::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/compat.rs:156:54
   4: <compio_tls::stream::TlsStream<S> as compio_io::write::AsyncWrite>::flush::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-tls/src/stream/mod.rs:165:44
   5: <compio_tls::stream::TlsStream<S> as compio_io::write::AsyncWrite>::shutdown::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-tls/src/stream/mod.rs:170:22
   6: send_wrapper::futures::<impl core::future::future::Future for send_wrapper::SendWrapper<F>>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/send_wrapper-0.6.0/src/futures.rs:27:3
   7: send_wrapper::futures::<impl core::future::future::Future for send_wrapper::SendWrapper<F>>::poll{{reify.shim}}
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/send_wrapper-0.6.0/src/futures.rs:23:2
   8: <cyper_core::stream::HyperStream<S> as hyper::rt::io::Write>::poll_shutdown
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cyper-core-0.1.0-beta.2/src/stream.rs:336:19
   9: <hyper_util::common::rewind::Rewind<T> as hyper::rt::io::Write>::poll_shutdown
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.3/src/common/rewind.rs:131:9
  10: hyper::proto::h1::conn::Conn<I,B,T>::poll_shutdown
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/conn.rs:765:22
  11: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_inner
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:154:24
  12: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_catch
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:126:28
  13: <hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:447:9
  14: <hyper::server::conn::http1::Connection<I,S> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/server/conn/http1.rs:215:22
  15: <hyper_util::server::conn::auto::Connection<I,S,E> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.3/src/server/conn/auto.rs:351:28
  16: example::main::{{closure}}
             at ./src/main.rs:60:18
  17: compio_runtime::runtime::RuntimeInner::block_on::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:92:68
  18: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /builddir/rust-1.76.0/library/core/src/future/future.rs:124:9
  19: async_task::raw::RawTask<F,T,S,M>::run
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/raw.rs:557:17
  20: async_task::runnable::Runnable<M>::run
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/runnable.rs:781:18
  21: compio_runtime::runtime::RuntimeInner::run
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:83:17
  22: compio_runtime::runtime::RuntimeInner::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:94:13
  23: compio_runtime::runtime::EnterGuard::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:496:9
  24: compio_runtime::runtime::Runtime::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:340:9
  25: example::main
             at ./src/main.rs:68:5
  26: core::ops::function::FnOnce::call_once
             at /builddir/rust-1.76.0/library/core/src/ops/function.rs:250:5
  27: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /builddir/rust-1.76.0/library/std/src/sys_common/backtrace.rs:155:18
  28: std::rt::lang_start::{{closure}}
             at /builddir/rust-1.76.0/library/std/src/rt.rs:166:18
  29: std::rt::lang_start_internal
  30: std::rt::lang_start
             at /builddir/rust-1.76.0/library/std/src/rt.rs:165:17
  31: main

stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: compio_io::buffer::Buffer::take_inner
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:205:17
   3: compio_io::buffer::Buffer::with::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:155:40
   4: compio_io::buffer::Buffer::flush_to::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/buffer.rs:179:18
   5: compio_io::compat::SyncStream<S>::flush_write_buf::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/compat.rs:156:54
   6: <compio_tls::stream::TlsStream<S> as compio_io::write::AsyncWrite>::flush::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-tls/src/stream/mod.rs:165:44
   7: compio_io::compat::SyncStream<S>::flush_write_buf::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-io/src/compat.rs:157:24
   8: send_wrapper::futures::<impl core::future::future::Future for send_wrapper::SendWrapper<F>>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/send_wrapper-0.6.0/src/futures.rs:27:3
   9: send_wrapper::futures::<impl core::future::future::Future for send_wrapper::SendWrapper<F>>::poll{{reify.shim}}
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/send_wrapper-0.6.0/src/futures.rs:23:2
  10: <cyper_core::stream::HyperStream<S> as hyper::rt::io::Write>::poll_flush
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cyper-core-0.1.0-beta.2/src/stream.rs:321:19
  11: <hyper_util::common::rewind::Rewind<T> as hyper::rt::io::Write>::poll_flush
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.3/src/common/rewind.rs:127:9
  12: hyper::proto::h1::io::Buffered<T,B>::poll_flush
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/io.rs:289:13
  13: hyper::proto::h1::conn::Conn<I,B,T>::poll_flush
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/conn.rs:758:16
  14: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_flush
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:400:9
  15: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:172:21
  16: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_inner
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:147:16
  17: hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_catch
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:126:28
  18: <hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/proto/h1/dispatch.rs:447:9
  19: <hyper::server::conn::http1::Connection<I,S> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-1.2.0/src/server/conn/http1.rs:215:22
  20: <hyper_util::server::conn::auto::Connection<I,S,E> as core::future::future::Future>::poll
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.3/src/server/conn/auto.rs:351:28
  21: example::main::{{closure}}
             at ./src/main.rs:60:18
  22: compio_runtime::runtime::RuntimeInner::block_on::{{closure}}
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:92:68
  23: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /builddir/rust-1.76.0/library/core/src/future/future.rs:124:9
  24: async_task::raw::RawTask<F,T,S,M>::run
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/raw.rs:557:17
  25: async_task::runnable::Runnable<M>::run
             at /home/val/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.0/src/runnable.rs:781:18
  26: compio_runtime::runtime::RuntimeInner::run
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:83:17
  27: compio_runtime::runtime::RuntimeInner::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:94:13
  28: compio_runtime::runtime::EnterGuard::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:496:9
  29: compio_runtime::runtime::Runtime::block_on
             at /home/val/src/github.com/compio-rs/compio/compio-runtime/src/runtime/mod.rs:340:9
  30: example::main
             at ./src/main.rs:68:5
  31: core::ops::function::FnOnce::call_once
             at /builddir/rust-1.76.0/library/core/src/ops/function.rs:250:5

With that I found that removing the shutdown → flush call got rid of the issue. Still that feels odd that it happens!

By the way, that TLS integration via sync stream traits looks a bit odd to me :eyes: I guess that's… fine? in a single-thread runtime…? but is there any reason in particular compio-tls isn't currently using futures-rustls and async-native-tls? (note: by "async-std" support async-native-tls really means futures-util)

Berrysoft commented 7 months ago

Good catch! However, I think it's a cyper bug, and should be fixed in 0.1.0-beta.2: https://github.com/compio-rs/cyper/commit/63dbdfeaf84068a39216482200463306959d8844

This PR is not right. shutdown method should call flush because the inner buffer should be flushed before the socket shutdown. However, if the flush method is still pending, shutdown should be called. That's the bug of HyperStream in the cyper repo, I think.

is there any reason in particular compio-tls isn't currently using futures-rustls and async-native-tls?

Yes. The sockets in compio is dealt very much differently to other crates, especially on Windows (IOCP) and Linux (io-uring).

Berrysoft commented 7 months ago

Oh, I understand. It is caused by a poll_flush call after poll_shutdown. I'll close this PR and open an issue in cyper repo.

valpackett commented 7 months ago

Leaving this around in case the debug instrumentation is useful to anyone (:

The sockets in compio is dealt very much differently to other crates, especially on Windows (IOCP) and Linux (io-uring).

hmm, but glommio does implement futures::io traits so futures-rustls does just work…

Berrysoft commented 7 months ago

but glommio does implement futures::io traits so futures-rustls does just work…

It uses a buffer inside the stream.