Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Ignore SIGPIPE #195

Open
lovesegfault opened this issue Jul 23, 2019 · 5 comments
Open

Ignore SIGPIPE #195

lovesegfault opened this issue Jul 23, 2019 · 5 comments

Comments

@lovesegfault
Copy link

I've been using tower-grpc at work, and after writing a test for a server/client comm I found that it would fail sporadically with a broken pipe error:

Protocol(Error(Io, Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }))

Now, the test would work, the data seemed to go through, and everything was fine if not for that annoying broken pipe. I used strace to see what was going on, and found the problem area:

12638 1563904381.031034 writev(79, [{iov_base="\0\24\274\0\0\0\0\0\1", iov_len=9}, {iov_base="\0\0\0\24\267\n\264)\f\0\0\0\10\0\f\0\7\0\10\0\10\0\0\0\0\0\0\1\4\0\0\0"..., iov_len=5308}], 2) = 5317
12644 1563904381.031084 close(78)             = 0
12643 1563904381.031094 sched_yield()         = 0
12639 1563904381.031122 sched_yield()         = 0
12642 1563904381.031147 epoll_wait(63, [{EPOLLIN, {u32=4294967295, u64=18446744073709551615}}], 1024, -1) = 1
12649 1563904381.031181 epoll_wait(40, [], 1024, 0) = 0
12643 1563904381.031191 sched_yield()         = 0
12641 1563904381.031198 sched_yield()         = 0
12639 1563904381.031237 sched_yield()         = 0
12643 1563904381.031300 epoll_wait(57, [], 1024, 0) = 0
12641 1563904381.031324 epoll_wait(64, [], 1024, 0) = 0
12644 1563904381.031362 write(66, "\1", 1)    = 1
12638 1563904381.031392 writev(79, [{iov_base="\0\0\f\1\5\0\0\0\1@\210\232\312\310\262\0224\332\217\201\7", iov_len=21}], 1) = -1 EPIPE (Broken pipe)

I will also note the flags set when opening the socket:

socket(AF_INET6, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_IP) = 78

I think this happens because, on the client side, the HTTP client is moved into the future that is passed onto the Tokio executor, which then drops the connection once that is completed. The canonical GRPC implementation circumvents this issue, I think, by ignoring SIGPIPE which I believe we should so as well.

Have I identified the issue correctly?

@seanmonstar
Copy link
Contributor

That PR to ignore SIGPIPE is to disable the sending of the SIGPIPE signal to the process, the EPIPE is still returned either way (signals just usually cause the whole process to abort).

What are you seeing the error from? From a specific RPC future? Or from the background connection future?

@lovesegfault
Copy link
Author

lovesegfault commented Jul 23, 2019

@seanmonstar The error, Protocol error: connection error: Broken pipe (os error 32) (I changed the message to help disambiguate it's origin) comes from the server code, which looks like this:

pub fn dummy_server<D>(db: D) {
    // See https://github.com/tower-rs/tower-grpc/issues/195
    unsafe {
        signal(Signal::SIGPIPE, SigHandler::SigIgn).unwrap();
    }

    let service = PipelineReadServiceServer::new(DbServer::new(db));
    let mut server = Server::new(service);
    let http = Http::new().http2_only(true).clone();
    let addr = "[::1]:50051".parse().unwrap();
    let bind = TcpListener::bind(&addr).expect(&format!("Bind failure at {:?}", addr));

    let serve = bind
        .incoming()
        .take(1)
        .for_each(move |sock| {
            if let Err(e) = sock.set_nodelay(true) {
                return Err(e);
            }
            let serve = server.serve_with(sock, http.clone());
            tokio::spawn(serve.map_err(|e| match e {
                Error::Protocol(e) => {
                    // Ignore SIGPIPE?
                    if e.kind() != hyper::error::Kind::Io {
                        eprintln!("Protocol error: {}", e)
                    }
                }
                _ => eprintln!("Serve error: {}", e),
            }));
            Ok(())
        })
        .map_err(|e| eprintln!("Accept error: {}", e));
    tokio::run(serve);
}

The client, in turn, looks like this

pub fn dummy_client() {
    let uri: http::Uri = format!("http://[::1]:50051").parse().unwrap();
    let dst = Destination::try_from_uri(uri.clone()).unwrap();
    let connector = util::Connector::new(HttpConnector::new(4));
    let settings = client::Builder::new().http2_only(true).clone();
    let mut make_client = client::Connect::with_builder(connector, settings);
    let reader = make_client
        .make_service(dst)
        .map_err(|e| panic!("Connection error: {:?}", e))
        .and_then(move |conn| {
            let conn = tower_request_modifier::Builder::new()
                .set_origin(uri)
                .build(conn)
                .unwrap();
            PipelineReadService::new(conn).ready()
        })
        .and_then(|mut client| {
            // Some unrelated code here
            client.range_read(Request::new(...))
        })
        .and_then(|_response| Ok(()))
        .map_err(|e| eprintln!("Error: {:?}", e));
    tokio::run(reader);
}

@lovesegfault
Copy link
Author

lovesegfault commented Jul 23, 2019

I'll add that the actual test procedure is this:

#[test]
fn basic_server_client_memdb() {
    let db = db::dummy_memdb();
    let server = std::thread::Builder::new()
        .name("dummy_grpc_server".to_owned())
        .spawn(|| server::dummy_server(db))
        .unwrap();
    let client = std::thread::Builder::new()
        .name("dummy_grpc_client".to_owned())
        .spawn(|| client::dummy_client())
        .unwrap();
    client.join().unwrap();
    server.join().unwrap();
}

@seanmonstar
Copy link
Contributor

So it appears that the server connection future itself is reporting the error. It could be trying to reply to the GOAWAY from the client.

(Note, you don't need to do anything about SIGPIPE signals, Rust disables them automatically in its prelude before main.)

@lovesegfault
Copy link
Author

I see, so I can just ignore the error (i.e. not print it out) and it's working as expected?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants