Inconsistent behaviour of one-shot servers on macOS and Linux

[glyn]

While investigating the intended behaviour of one-shot servers, I found an inconsistency between macOS and Linux. I can't tell if the inconsistency is intentional (seems unlikely) or whether the behaviour should be consistent and match the behaviour on macOS or Linux or something else.

The inconsistency is as follows. If multiple client processes connect to a one-shot server and send requests to it, the server can receive all the requests on macOS, but can only receive the requests from one client on Linux.

I think what's happening on macOS is that each client process's connect request accesses the same unidirectional communication channel (described here) associated with the Mach port underlying the one-shot server and so the clients' requests all make their way to the channel resulting from the server's call to accept().

I think what's happening on Linux is that each client process's connect request would result in a distinct socket connection returned from the socket accept() underpinning one-shot server's accept(). However, because one-shot server's accept() can only be called once, only the first client connect request is processed and other connect requests wait indefinitely to be accepted.

To make this concrete, the following test passes on macOS, but fails on Linux:

#[cfg(not(any(
    feature = "force-inprocess",
    target_os = "windows",
    target_os = "android",
    target_os = "ios"
)))]
#[test]
fn cross_process_connecting_multiple_times_to_one_shot_server() {
    let person = ("Arthur Dent".to_owned(), 29);
    let person2 = ("Ford Prefect".to_owned(), 54);
    let (server0, server0_name) = IpcOneShotServer::new().unwrap();
    let child_pid = unsafe {
        fork(|| {
            let tx0 = IpcSender::connect(server0_name.clone()).unwrap();
            tx0.send(person.clone()).unwrap();
        })
    };
    let child_pid2 = unsafe {
        fork(|| {
            let tx0 = IpcSender::connect(server0_name.clone()).unwrap();
            tx0.send(person2.clone()).unwrap();
        })
    };
    let (rx, received_person): (IpcReceiver<Person>, Person) = server0.accept().unwrap();
    let received_person2 = rx.recv().unwrap(); // This panics with "`Err`` value: Disconnected" on Linux
    child_pid.wait();
    child_pid2.wait();
    assert_eq!(received_person, person);
    assert_eq!(received_person2, person2);
}

I am raising this issue to clarify the intended behaviour and potentially to initiate a code change to make the behaviour consistent.

[jdm]

I suspect the intention here is that a one-shot server can only be used to allow one client connection, but I need to check how Servo uses them right now.

[glyn]

Thanks. It would be helpful to know the current Servo usage of one-shot servers, especially if there are platform-specific variations in that usage.

Note that allowing only one client connection (the current behaviour on Unix variants) is race-prone. Essentially the first client to call connect() and have its connection request accepted wins (can send requests to the server) and clients subsequently calling connect() lose (their connections are never accepted and any requests they send are never received). When two or more connect() requests are issued concurrently (e.g. from distinct processes or threads), the race becomes more likely.

[jdm]

Verified that Servo only uses one shot servers once, regardless of platform: https://github.com/servo/servo/blob/e74539404467cc31c03f52a419371258235a1d69/components/constellation/sandboxing.rs#L230 . We avoid races by only providing the token to a single process, so there are never multiple clients attempting to connect.

[glyn]

Ok, that's good.

Other users of ipc-channel, and perhaps Servo in the future, could run into the issue. So I suggest:

• Short term: document the behaviour of connect (I'll submit a PR).
• Medium term: explore ways of making the behaviour consistent (and sensible), e.g. by causing the second and subsequent attempts to connect to a one-shot server to fail, regardless of platform.
• Longer term: explore a different programming model which avoids exposing the token and connect(), and instead provides a way of obtaining a sender to the server once and only once.

[glyn]

I submitted draft PR #380 for review and comment.

I couldn't see a reliable way to cause second and subsequent calls to connect() to fail for a given one-shot server, so this is an attempt to upgrade the programming model. The approach is to deprecate new() and connect() and then delete them in a later release.