Don't acquire tokens on inherited jobservers

After some thinking and some testing, this seems like it was just always a bad
idea unfortunately! I think that this basically ends up just always leading to
deadlock, so this patch attempts to work around this behavior by avoiding
acquiring any tokens whenever there's an inherited jobserver.

As to why I think this is good long-term behavior, it's probably useful to dig
into what's happening right now. Currently if sccache ever creates it's own
jobserver it should be properly acquiring tokens from it and the processes
spawned should be configured/return tokens and such. This, as far as I know,
isn't the case to worry about.

Instead, we're exclusively worried about two situations: one where the server
itself inherits a jobserver and one where the client sccache process inherits a
jobserver. In the former case things go very wrong very quickly. All clients, as
part of the build system, typically request a jobserver token before running any
code. This means that the client process acquired a token *and* the server
process is going to attempt to acquire a token.

For example consider a jobserver of 4 tokens. If our server is sitting idle we
may spawn 4 processes, acquiring four tokens. All our clients now request the
server to do some work, which *also* requires four more tokens to proceed, so
deadlock!

The next case we're worried about is when the server has its own jobserver but
the clients inherited theirs from the ambient build system. While this doesn't
happen in `make` unless explicitly specified, this happens commonly in Cargo.
Here the client *also* requests a jobserver token to spawn a process for
unhandled compiles. This is mostly done to make the code a bit cleaner for now
but results in the same deadlock we had before.

So all in all, all signs point to acquiring tokens when you inherited a
jobserver as a bad idea. This commit changes the jobserver in sccache to, when
inheriting from the environment, never acquire tokens. This means that all the
tokens acquired to spawn processes are pseudo transferred to the server as the
server does all the work instead of the client. We still configure all
subprocesses to have the fds, however.

Hopefully this...

Closes #221

src/jobserver.rs

@@ -5,26 +5,29 @@ use std::process::Command;
 use std::sync::Arc;
 
 use futures::prelude::*;
+use futures::future;
 use futures::sync::mpsc;
 use futures::sync::oneshot;
 use num_cpus;
 
 use errors::*;
 
-pub use self::jobserver::Acquired;
-
 #[derive(Clone)]
 pub struct Client {
-    helper: Arc<jobserver::HelperThread>,
+    helper: Option<Arc<jobserver::HelperThread>>,
+    tx: Option<mpsc::UnboundedSender<oneshot::Sender<io::Result<jobserver::Acquired>>>>,
     inner: jobserver::Client,
-    tx: mpsc::UnboundedSender<oneshot::Sender<io::Result<Acquired>>>
+}
+
+pub struct Acquired {
+    _token: Option<jobserver::Acquired>,
 }
 
 impl Client {
     // unsafe because `from_env` is unsafe (can use the wrong fds)
     pub unsafe fn new() -> Client {
         match jobserver::Client::from_env() {
-            Some(c) => Client::_new(c),
+            Some(c) => Client::_new(c, true),
             None => Client::new_num(num_cpus::get()),
         }
     }
@@ -32,23 +35,24 @@ impl Client {
     pub fn new_num(num: usize) -> Client {
         let inner = jobserver::Client::new(num)
                 .expect("failed to create jobserver");
-        Client::_new(inner)
+        Client::_new(inner, false)
     }
 
-    fn _new(inner: jobserver::Client) -> Client {
-        let (tx, rx) = mpsc::unbounded::<oneshot::Sender<_>>();
-        let mut rx = rx.wait();
-        let helper = inner.clone().into_helper_thread(move |token| {
-            if let Some(Ok(sender)) = rx.next() {
-                drop(sender.send(token));
-            }
-        }).expect("failed to spawn helper thread");
+    fn _new(inner: jobserver::Client, inherited: bool) -> Client {
+        let (helper, tx) = if inherited {
+            (None, None)
+        } else {
+            let (tx, rx) = mpsc::unbounded::<oneshot::Sender<_>>();
+            let mut rx = rx.wait();
+            let helper = inner.clone().into_helper_thread(move |token| {
+                if let Some(Ok(sender)) = rx.next() {
+                    drop(sender.send(token));
+                }
+            }).expect("failed to spawn helper thread");
+            (Some(Arc::new(helper)), Some(tx))
+        };
 
-        Client {
-            inner: inner,
-            helper: Arc::new(helper),
-            tx: tx,
-        }
+        Client { inner, helper, tx }
     }
 
     /// Configures this jobserver to be inherited by the specified command
@@ -62,10 +66,15 @@ impl Client {
     /// defnition of "work" is) to ensure that the system is properly
     /// rate-limiting itself.
     pub fn acquire(&self) -> SFuture<Acquired> {
-        let (tx, rx) = oneshot::channel();
-        self.helper.request_token();
-        self.tx.unbounded_send(tx).unwrap();
-        Box::new(rx.chain_err(|| "jobserver helper panicked")
-                   .and_then(|t| t.chain_err(|| "failed to acquire jobserver token")))
+        let (helper, tx) = match (self.helper.as_ref(), self.tx.as_ref()) {
+            (Some(a), Some(b)) => (a, b),
+            _ => return Box::new(future::ok(Acquired { _token: None })),
+        };
+        let (mytx, myrx) = oneshot::channel();
+        helper.request_token();
+        tx.unbounded_send(mytx).unwrap();
+        Box::new(myrx.chain_err(|| "jobserver helper panicked")
+                   .and_then(|t| t.chain_err(|| "failed to acquire jobserver token"))
+                   .map(|t| Acquired { _token: Some(t) }))
     }
 }