path: root/tools/ninja/patches/100-make_jobserver_support.patch

From 17d13fd7881fd3ce9f9b9d44ce435d6caf4b8f28 Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefanb@gpartner-nvidia.com>
Date: Tue, 22 Mar 2016 13:48:07 +0200
Subject: [PATCH 01/11] Add GNU make jobserver client support

- add new TokenPool interface
- GNU make implementation for TokenPool parses and verifies the magic
  information from the MAKEFLAGS environment variable
- RealCommandRunner tries to acquire TokenPool
  * if no token pool is available then there is no change in behaviour
- When a token pool is available then RealCommandRunner behaviour
  changes as follows
  * CanRunMore() only returns true if TokenPool::Acquire() returns true
  * StartCommand() calls TokenPool::Reserve()
  * WaitForCommand() calls TokenPool::Release()

Documentation for GNU make jobserver

  http://make.mad-scientist.net/papers/jobserver-implementation/

Fixes https://github.com/ninja-build/ninja/issues/1139
---
 configure.py              |   2 +
 src/build.cc              |  63 ++++++++----
 src/build.h               |   3 +
 src/tokenpool-gnu-make.cc | 211 ++++++++++++++++++++++++++++++++++++++
 src/tokenpool-none.cc     |  27 +++++
 src/tokenpool.h           |  26 +++++
 6 files changed, 310 insertions(+), 22 deletions(-)
 create mode 100644 src/tokenpool-gnu-make.cc
 create mode 100644 src/tokenpool-none.cc
 create mode 100644 src/tokenpool.h

diff --git a/configure.py b/configure.py
index 43904349a8..db3492c93c 100755
--- a/configure.py
+++ b/configure.py
@@ -522,6 +522,7 @@ def has_re2c():
     objs += cxx(name, variables=cxxvariables)
 if platform.is_windows():
     for name in ['subprocess-win32',
+                 'tokenpool-none',
                  'includes_normalize-win32',
                  'msvc_helper-win32',
                  'msvc_helper_main-win32']:
@@ -531,6 +532,7 @@ def has_re2c():
     objs += cc('getopt')
 else:
     objs += cxx('subprocess-posix')
+    objs += cxx('tokenpool-gnu-make')
 if platform.is_aix():
     objs += cc('getopt')
 if platform.is_msvc():
diff --git a/src/build.cc b/src/build.cc
index 6f11ed7a3c..fa096eac33 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -35,6 +35,7 @@
 #include "state.h"
 #include "status.h"
 #include "subprocess.h"
+#include "tokenpool.h"
 #include "util.h"
 
 using namespace std;
@@ -149,7 +150,7 @@ void Plan::EdgeWanted(const Edge* edge) {
 }
 
 Edge* Plan::FindWork() {
-  if (ready_.empty())
+  if (!more_ready())
     return NULL;
   EdgeSet::iterator e = ready_.begin();
   Edge* edge = *e;
@@ -448,8 +449,8 @@ void Plan::Dump() const {
 }
 
 struct RealCommandRunner : public CommandRunner {
-  explicit RealCommandRunner(const BuildConfig& config) : config_(config) {}
-  virtual ~RealCommandRunner() {}
+  explicit RealCommandRunner(const BuildConfig& config);
+  virtual ~RealCommandRunner();
   virtual bool CanRunMore() const;
   virtual bool StartCommand(Edge* edge);
   virtual bool WaitForCommand(Result* result);
@@ -458,9 +459,18 @@ struct RealCommandRunner : public CommandRunner {
 
   const BuildConfig& config_;
   SubprocessSet subprocs_;
+  TokenPool *tokens_;
   map<const Subprocess*, Edge*> subproc_to_edge_;
 };
 
+RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
+  tokens_ = TokenPool::Get();
+}
+
+RealCommandRunner::~RealCommandRunner() {
+  delete tokens_;
+}
+
 vector<Edge*> RealCommandRunner::GetActiveEdges() {
   vector<Edge*> edges;
   for (map<const Subprocess*, Edge*>::iterator e = subproc_to_edge_.begin();
@@ -471,14 +481,18 @@ vector<Edge*> RealCommandRunner::GetActiveEdges() {
 
 void RealCommandRunner::Abort() {
   subprocs_.Clear();
+  if (tokens_)
+    tokens_->Clear();
 }
 
 bool RealCommandRunner::CanRunMore() const {
   size_t subproc_number =
       subprocs_.running_.size() + subprocs_.finished_.size();
   return (int)subproc_number < config_.parallelism
-    && ((subprocs_.running_.empty() || config_.max_load_average <= 0.0f)
-        || GetLoadAverage() < config_.max_load_average);
+    && (subprocs_.running_.empty() ||
+        ((config_.max_load_average <= 0.0f ||
+          GetLoadAverage() < config_.max_load_average)
+      && (!tokens_ || tokens_->Acquire())));
 }
 
 bool RealCommandRunner::StartCommand(Edge* edge) {
@@ -486,6 +500,8 @@ bool RealCommandRunner::StartCommand(Edge* edge) {
   Subprocess* subproc = subprocs_.Add(command, edge->use_console());
   if (!subproc)
     return false;
+  if (tokens_)
+    tokens_->Reserve();
   subproc_to_edge_.insert(make_pair(subproc, edge));
 
   return true;
@@ -499,6 +515,9 @@ bool RealCommandRunner::WaitForCommand(Result* result) {
       return false;
   }
 
+  if (tokens_)
+    tokens_->Release();
+
   result->status = subproc->Finish();
   result->output = subproc->GetOutput();
 
@@ -621,31 +640,31 @@ bool Builder::Build(string* err) {
   // Second, we attempt to wait for / reap the next finished command.
   while (plan_.more_to_do()) {
     // See if we can start any more commands.
-    if (failures_allowed && command_runner_->CanRunMore()) {
-      if (Edge* edge = plan_.FindWork()) {
-        if (edge->GetBindingBool("generator")) {
+    if (failures_allowed && plan_.more_ready() &&
+        command_runner_->CanRunMore()) {
+      Edge* edge = plan_.FindWork();
+      if (edge->GetBindingBool("generator")) {
           scan_.build_log()->Close();
         }
 
-        if (!StartEdge(edge, err)) {
+      if (!StartEdge(edge, err)) {
+        Cleanup();
+        status_->BuildFinished();
+        return false;
+      }
+
+      if (edge->is_phony()) {
+        if (!plan_.EdgeFinished(edge, Plan::kEdgeSucceeded, err)) {
           Cleanup();
           status_->BuildFinished();
           return false;
         }
-
-        if (edge->is_phony()) {
-          if (!plan_.EdgeFinished(edge, Plan::kEdgeSucceeded, err)) {
-            Cleanup();
-            status_->BuildFinished();
-            return false;
-          }
-        } else {
-          ++pending_commands;
-        }
-
-        // We made some progress; go back to the main loop.
-        continue;
+      } else {
+        ++pending_commands;
       }
+
+      // We made some progress; go back to the main loop.
+      continue;
     }
 
     // See if we can reap any finished commands.
diff --git a/src/build.h b/src/build.h
index d697dfb89e..7dcd111e61 100644
--- a/src/build.h
+++ b/src/build.h
@@ -52,6 +52,9 @@ struct Plan {
   /// Returns true if there's more work to be done.
   bool more_to_do() const { return wanted_edges_ > 0 && command_edges_ > 0; }
 
+  /// Returns true if there's more edges ready to start
+  bool more_ready() const { return !ready_.empty(); }
+
   /// Dumps the current state of the plan.
   void Dump() const;
 
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
new file mode 100644
index 0000000000..a8f9b7139d
--- /dev/null
+++ b/src/tokenpool-gnu-make.cc
@@ -0,0 +1,211 @@
+// Copyright 2016 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <poll.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+// TokenPool implementation for GNU make jobserver
+// (http://make.mad-scientist.net/papers/jobserver-implementation/)
+struct GNUmakeTokenPool : public TokenPool {
+  GNUmakeTokenPool();
+  virtual ~GNUmakeTokenPool();
+
+  virtual bool Acquire();
+  virtual void Reserve();
+  virtual void Release();
+  virtual void Clear();
+
+  bool Setup();
+
+ private:
+  int available_;
+  int used_;
+
+#ifdef _WIN32
+  // @TODO
+#else
+  int rfd_;
+  int wfd_;
+
+  struct sigaction old_act_;
+  bool restore_;
+
+  static int dup_rfd_;
+  static void CloseDupRfd(int signum);
+
+  bool CheckFd(int fd);
+  bool SetAlarmHandler();
+#endif
+
+  void Return();
+};
+
+// every instance owns an implicit token -> available_ == 1
+GNUmakeTokenPool::GNUmakeTokenPool() : available_(1), used_(0),
+                                       rfd_(-1), wfd_(-1), restore_(false) {
+}
+
+GNUmakeTokenPool::~GNUmakeTokenPool() {
+  Clear();
+  if (restore_)
+    sigaction(SIGALRM, &old_act_, NULL);
+}
+
+bool GNUmakeTokenPool::CheckFd(int fd) {
+  if (fd < 0)
+    return false;
+  int ret = fcntl(fd, F_GETFD);
+  if (ret < 0)
+    return false;
+  return true;
+}
+
+int GNUmakeTokenPool::dup_rfd_ = -1;
+
+void GNUmakeTokenPool::CloseDupRfd(int signum) {
+  close(dup_rfd_);
+  dup_rfd_ = -1;
+}
+
+bool GNUmakeTokenPool::SetAlarmHandler() {
+  struct sigaction act;
+  memset(&act, 0, sizeof(act));
+  act.sa_handler = CloseDupRfd;
+  if (sigaction(SIGALRM, &act, &old_act_) < 0) {
+    perror("sigaction:");
+    return(false);
+  } else {
+    restore_ = true;
+    return(true);
+  }
+}
+
+bool GNUmakeTokenPool::Setup() {
+  const char *value = getenv("MAKEFLAGS");
+  if (value) {
+    // GNU make <= 4.1
+    const char *jobserver = strstr(value, "--jobserver-fds=");
+    // GNU make => 4.2
+    if (!jobserver)
+      jobserver = strstr(value, "--jobserver-auth=");
+    if (jobserver) {
+      int rfd = -1;
+      int wfd = -1;
+      if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
+          CheckFd(rfd) &&
+          CheckFd(wfd) &&
+          SetAlarmHandler()) {
+        printf("ninja: using GNU make jobserver.\n");
+        rfd_ = rfd;
+        wfd_ = wfd;
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
+bool GNUmakeTokenPool::Acquire() {
+  if (available_ > 0)
+    return true;
+
+#ifdef USE_PPOLL
+  pollfd pollfds[] = {{rfd_, POLLIN, 0}};
+  int ret = poll(pollfds, 1, 0);
+#else
+  fd_set set;
+  struct timeval timeout = { 0, 0 };
+  FD_ZERO(&set);
+  FD_SET(rfd_, &set);
+  int ret = select(rfd_ + 1, &set, NULL, NULL, &timeout);
+#endif
+  if (ret > 0) {
+    dup_rfd_ = dup(rfd_);
+
+    if (dup_rfd_ != -1) {
+      struct sigaction act, old_act;
+      int ret = 0;
+
+      memset(&act, 0, sizeof(act));
+      act.sa_handler = CloseDupRfd;
+      if (sigaction(SIGCHLD, &act, &old_act) == 0) {
+        char buf;
+
+        // block until token read, child exits or timeout
+        alarm(1);
+        ret = read(dup_rfd_, &buf, 1);
+        alarm(0);
+
+        sigaction(SIGCHLD, &old_act, NULL);
+      }
+
+      CloseDupRfd(0);
+
+      if (ret > 0) {
+        available_++;
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+void GNUmakeTokenPool::Reserve() {
+  available_--;
+  used_++;
+}
+
+void GNUmakeTokenPool::Return() {
+  const char buf = '+';
+  while (1) {
+    int ret = write(wfd_, &buf, 1);
+    if (ret > 0)
+      available_--;
+    if ((ret != -1) || (errno != EINTR))
+      return;
+    // write got interrupted - retry
+  }
+}
+
+void GNUmakeTokenPool::Release() {
+  available_++;
+  used_--;
+  if (available_ > 1)
+    Return();
+}
+
+void GNUmakeTokenPool::Clear() {
+  while (used_ > 0)
+    Release();
+  while (available_ > 1)
+    Return();
+}
+
+struct TokenPool *TokenPool::Get(void) {
+  GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
+  if (tokenpool->Setup())
+    return tokenpool;
+  else
+    delete tokenpool;
+  return NULL;
+}
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
new file mode 100644
index 0000000000..602b3316f5
--- /dev/null
+++ b/src/tokenpool-none.cc
@@ -0,0 +1,27 @@
+// Copyright 2016 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool.h"
+
+#include <fcntl.h>
+#include <poll.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+// No-op TokenPool implementation
+struct TokenPool *TokenPool::Get(void) {
+  return NULL;
+}
diff --git a/src/tokenpool.h b/src/tokenpool.h
new file mode 100644
index 0000000000..f560b1083b
--- /dev/null
+++ b/src/tokenpool.h
@@ -0,0 +1,26 @@
+// Copyright 2016 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// interface to token pool
+struct TokenPool {
+  virtual ~TokenPool() {}
+
+  virtual bool Acquire() = 0;
+  virtual void Reserve() = 0;
+  virtual void Release() = 0;
+  virtual void Clear() = 0;
+
+  // returns NULL if token pool is not available
+  static struct TokenPool *Get(void);
+};

From ccaccc610cd456f6068758f82e72006364c7380b Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefanb@gpartner-nvidia.com>
Date: Fri, 27 May 2016 16:47:10 +0300
Subject: [PATCH 02/11] Add TokenPool monitoring to SubprocessSet::DoWork()

Improve on the original jobserver client implementation. This makes
ninja a more aggressive GNU make jobserver client.

- add monitor interface to TokenPool
- TokenPool is passed down when main loop indicates that more work is
  ready and would be allowed to start if a token becomes available
- posix: update DoWork() to monitor TokenPool read file descriptor
- WaitForCommand() exits when DoWork() sets token flag
- Main loop starts over when WaitForCommand() sets token exit status
---
 src/build.cc              | 53 +++++++++++++++++++++++++++++----------
 src/build.h               |  3 ++-
 src/build_test.cc         |  9 +++++--
 src/exit_status.h         |  3 ++-
 src/subprocess-posix.cc   | 33 ++++++++++++++++++++++--
 src/subprocess-win32.cc   |  2 +-
 src/subprocess.h          |  8 +++++-
 src/subprocess_test.cc    | 47 +++++++++++++++++++++++-----------
 src/tokenpool-gnu-make.cc |  5 ++++
 src/tokenpool.h           |  6 +++++
 10 files changed, 134 insertions(+), 35 deletions(-)

diff --git a/src/build.cc b/src/build.cc
index fa096eac33..a25c349050 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -48,8 +48,9 @@ struct DryRunCommandRunner : public CommandRunner {
 
   // Overridden from CommandRunner:
   virtual bool CanRunMore() const;
+  virtual bool AcquireToken();
   virtual bool StartCommand(Edge* edge);
-  virtual bool WaitForCommand(Result* result);
+  virtual bool WaitForCommand(Result* result, bool more_ready);
 
  private:
   queue<Edge*> finished_;
@@ -59,12 +60,16 @@ bool DryRunCommandRunner::CanRunMore() const {
   return true;
 }
 
+bool DryRunCommandRunner::AcquireToken() {
+  return true;
+}
+
 bool DryRunCommandRunner::StartCommand(Edge* edge) {
   finished_.push(edge);
   return true;
 }
 
-bool DryRunCommandRunner::WaitForCommand(Result* result) {
+bool DryRunCommandRunner::WaitForCommand(Result* result, bool more_ready) {
    if (finished_.empty())
      return false;
 
@@ -452,8 +457,9 @@ struct RealCommandRunner : public CommandRunner {
   explicit RealCommandRunner(const BuildConfig& config);
   virtual ~RealCommandRunner();
   virtual bool CanRunMore() const;
+  virtual bool AcquireToken();
   virtual bool StartCommand(Edge* edge);
-  virtual bool WaitForCommand(Result* result);
+  virtual bool WaitForCommand(Result* result, bool more_ready);
   virtual vector<Edge*> GetActiveEdges();
   virtual void Abort();
 
@@ -490,9 +496,12 @@ bool RealCommandRunner::CanRunMore() const {
       subprocs_.running_.size() + subprocs_.finished_.size();
   return (int)subproc_number < config_.parallelism
     && (subprocs_.running_.empty() ||
-        ((config_.max_load_average <= 0.0f ||
-          GetLoadAverage() < config_.max_load_average)
-      && (!tokens_ || tokens_->Acquire())));
+        (config_.max_load_average <= 0.0f ||
+         GetLoadAverage() < config_.max_load_average));
+}
+
+bool RealCommandRunner::AcquireToken() {
+  return (!tokens_ || tokens_->Acquire());
 }
 
 bool RealCommandRunner::StartCommand(Edge* edge) {
@@ -507,14 +516,23 @@ bool RealCommandRunner::StartCommand(Edge* edge) {
   return true;
 }
 
-bool RealCommandRunner::WaitForCommand(Result* result) {
+bool RealCommandRunner::WaitForCommand(Result* result, bool more_ready) {
   Subprocess* subproc;
-  while ((subproc = subprocs_.NextFinished()) == NULL) {
-    bool interrupted = subprocs_.DoWork();
+  subprocs_.ResetTokenAvailable();
+  while (((subproc = subprocs_.NextFinished()) == NULL) &&
+         !subprocs_.IsTokenAvailable()) {
+    bool interrupted = subprocs_.DoWork(more_ready ? tokens_ : NULL);
     if (interrupted)
       return false;
   }
 
+  // token became available
+  if (subproc == NULL) {
+    result->status = ExitTokenAvailable;
+    return true;
+  }
+
+  // command completed
   if (tokens_)
     tokens_->Release();
 
@@ -639,9 +657,14 @@ bool Builder::Build(string* err) {
   // command runner.
   // Second, we attempt to wait for / reap the next finished command.
   while (plan_.more_to_do()) {
-    // See if we can start any more commands.
-    if (failures_allowed && plan_.more_ready() &&
-        command_runner_->CanRunMore()) {
+    // See if we can start any more commands...
+    bool can_run_more =
+        failures_allowed   &&
+        plan_.more_ready() &&
+        command_runner_->CanRunMore();
+
+    // ... but we also need a token to do that.
+    if (can_run_more && command_runner_->AcquireToken()) {
       Edge* edge = plan_.FindWork();
       if (edge->GetBindingBool("generator")) {
           scan_.build_log()->Close();
@@ -670,7 +693,7 @@ bool Builder::Build(string* err) {
     // See if we can reap any finished commands.
     if (pending_commands) {
       CommandRunner::Result result;
-      if (!command_runner_->WaitForCommand(&result) ||
+      if (!command_runner_->WaitForCommand(&result, can_run_more) ||
           result.status == ExitInterrupted) {
         Cleanup();
         status_->BuildFinished();
@@ -678,6 +701,10 @@ bool Builder::Build(string* err) {
         return false;
       }
 
+      // We might be able to start another command; start the main loop over.
+      if (result.status == ExitTokenAvailable)
+        continue;
+
       --pending_commands;
       if (!FinishCommand(&result, err)) {
         Cleanup();
diff --git a/src/build.h b/src/build.h
index 7dcd111e61..35c7b97d12 100644
--- a/src/build.h
+++ b/src/build.h
@@ -139,6 +139,7 @@ struct Plan {
 struct CommandRunner {
   virtual ~CommandRunner() {}
   virtual bool CanRunMore() const = 0;
+  virtual bool AcquireToken() = 0;
   virtual bool StartCommand(Edge* edge) = 0;
 
   /// The result of waiting for a command.
@@ -150,7 +151,7 @@ struct CommandRunner {
     bool success() const { return status == ExitSuccess; }
   };
   /// Wait for a command to complete, or return false if interrupted.
-  virtual bool WaitForCommand(Result* result) = 0;
+  virtual bool WaitForCommand(Result* result, bool more_ready) = 0;
 
   virtual std::vector<Edge*> GetActiveEdges() { return std::vector<Edge*>(); }
   virtual void Abort() {}
diff --git a/src/build_test.cc b/src/build_test.cc
index 4ef62b2113..7a5ff4015a 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -474,8 +474,9 @@ struct FakeCommandRunner : public CommandRunner {
 
   // CommandRunner impl
   virtual bool CanRunMore() const;
+  virtual bool AcquireToken();
   virtual bool StartCommand(Edge* edge);
-  virtual bool WaitForCommand(Result* result);
+  virtual bool WaitForCommand(Result* result, bool more_ready);
   virtual vector<Edge*> GetActiveEdges();
   virtual void Abort();
 
@@ -578,6 +579,10 @@ bool FakeCommandRunner::CanRunMore() const {
   return active_edges_.size() < max_active_edges_;
 }
 
+bool FakeCommandRunner::AcquireToken() {
+  return true;
+}
+
 bool FakeCommandRunner::StartCommand(Edge* edge) {
   assert(active_edges_.size() < max_active_edges_);
   assert(find(active_edges_.begin(), active_edges_.end(), edge)
@@ -649,7 +654,7 @@ bool FakeCommandRunner::StartCommand(Edge* edge) {
   return true;
 }
 
-bool FakeCommandRunner::WaitForCommand(Result* result) {
+bool FakeCommandRunner::WaitForCommand(Result* result, bool more_ready) {
   if (active_edges_.empty())
     return false;
 
diff --git a/src/exit_status.h b/src/exit_status.h
index a714ece791..75ebf6a7a0 100644
--- a/src/exit_status.h
+++ b/src/exit_status.h
@@ -18,7 +18,8 @@
 enum ExitStatus {
   ExitSuccess,
   ExitFailure,
-  ExitInterrupted
+  ExitTokenAvailable,
+  ExitInterrupted,
 };
 
 #endif  // NINJA_EXIT_STATUS_H_
diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc
index 8e785406c9..74451b0be2 100644
--- a/src/subprocess-posix.cc
+++ b/src/subprocess-posix.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "subprocess.h"
+#include "tokenpool.h"
 
 #include <sys/select.h>
 #include <assert.h>
@@ -249,7 +250,7 @@ Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
 }
 
 #ifdef USE_PPOLL
-bool SubprocessSet::DoWork() {
+bool SubprocessSet::DoWork(struct TokenPool* tokens) {
   vector<pollfd> fds;
   nfds_t nfds = 0;
 
@@ -263,6 +264,12 @@ bool SubprocessSet::DoWork() {
     ++nfds;
   }
 
+  if (tokens) {
+    pollfd pfd = { tokens->GetMonitorFd(), POLLIN | POLLPRI, 0 };
+    fds.push_back(pfd);
+    ++nfds;
+  }
+
   interrupted_ = 0;
   int ret = ppoll(&fds.front(), nfds, NULL, &old_mask_);
   if (ret == -1) {
@@ -295,11 +302,20 @@ bool SubprocessSet::DoWork() {
     ++i;
   }
 
+  if (tokens) {
+    pollfd *pfd = &fds[nfds - 1];
+    if (pfd->fd >= 0) {
+      assert(pfd->fd == tokens->GetMonitorFd());
+      if (pfd->revents != 0)
+        token_available_ = true;
+    }
+  }
+
   return IsInterrupted();
 }
 
 #else  // !defined(USE_PPOLL)
-bool SubprocessSet::DoWork() {
+bool SubprocessSet::DoWork(struct TokenPool* tokens) {
   fd_set set;
   int nfds = 0;
   FD_ZERO(&set);
@@ -314,6 +330,13 @@ bool SubprocessSet::DoWork() {
     }
   }
 
+  if (tokens) {
+    int fd = tokens->GetMonitorFd();
+    FD_SET(fd, &set);
+    if (nfds < fd+1)
+      nfds = fd+1;
+  }
+
   interrupted_ = 0;
   int ret = pselect(nfds, &set, 0, 0, 0, &old_mask_);
   if (ret == -1) {
@@ -342,6 +365,12 @@ bool SubprocessSet::DoWork() {
     ++i;
   }
 
+  if (tokens) {
+    int fd = tokens->GetMonitorFd();
+    if ((fd >= 0) && FD_ISSET(fd, &set))
+    token_available_ = true;
+  }
+
   return IsInterrupted();
 }
 #endif  // !defined(USE_PPOLL)
diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc
index ff3baaca7f..66d2c2c430 100644
--- a/src/subprocess-win32.cc
+++ b/src/subprocess-win32.cc
@@ -251,7 +251,7 @@ Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
   return subprocess;
 }
 
-bool SubprocessSet::DoWork() {
+bool SubprocessSet::DoWork(struct TokenPool* tokens) {
   DWORD bytes_read;
   Subprocess* subproc;
   OVERLAPPED* overlapped;
diff --git a/src/subprocess.h b/src/subprocess.h
index 9e3d2ee98f..9ea67ea477 100644
--- a/src/subprocess.h
+++ b/src/subprocess.h
@@ -76,6 +76,8 @@ struct Subprocess {
   friend struct SubprocessSet;
 };
 
+struct TokenPool;
+
 /// SubprocessSet runs a ppoll/pselect() loop around a set of Subprocesses.
 /// DoWork() waits for any state change in subprocesses; finished_
 /// is a queue of subprocesses as they finish.
@@ -84,13 +86,17 @@ struct SubprocessSet {
   ~SubprocessSet();
 
   Subprocess* Add(const std::string& command, bool use_console = false);
-  bool DoWork();
+  bool DoWork(struct TokenPool* tokens);
   Subprocess* NextFinished();
   void Clear();
 
   std::vector<Subprocess*> running_;
   std::queue<Subprocess*> finished_;
 
+  bool token_available_;
+  bool IsTokenAvailable() { return token_available_; }
+  void ResetTokenAvailable() { token_available_ = false; }
+
 #ifdef _WIN32
   static BOOL WINAPI NotifyInterrupted(DWORD dwCtrlType);
   static HANDLE ioport_;
diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
index 073fe86931..4bc8083e26 100644
--- a/src/subprocess_test.cc
+++ b/src/subprocess_test.cc
@@ -45,10 +45,12 @@ TEST_F(SubprocessTest, BadCommandStderr) {
   Subprocess* subproc = subprocs_.Add("cmd /c ninja_no_such_command");
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
     // Pretend we discovered that stderr was ready for writing.
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
@@ -59,10 +61,12 @@ TEST_F(SubprocessTest, NoSuchCommand) {
   Subprocess* subproc = subprocs_.Add("ninja_no_such_command");
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
     // Pretend we discovered that stderr was ready for writing.
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
@@ -78,9 +82,11 @@ TEST_F(SubprocessTest, InterruptChild) {
   Subprocess* subproc = subprocs_.Add("kill -INT $$");
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -90,7 +96,7 @@ TEST_F(SubprocessTest, InterruptParent) {
   ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
-    bool interrupted = subprocs_.DoWork();
+    bool interrupted = subprocs_.DoWork(NULL);
     if (interrupted)
       return;
   }
@@ -102,9 +108,11 @@ TEST_F(SubprocessTest, InterruptChildWithSigTerm) {
   Subprocess* subproc = subprocs_.Add("kill -TERM $$");
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -114,7 +122,7 @@ TEST_F(SubprocessTest, InterruptParentWithSigTerm) {
   ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
-    bool interrupted = subprocs_.DoWork();
+    bool interrupted = subprocs_.DoWork(NULL);
     if (interrupted)
       return;
   }
@@ -126,9 +134,11 @@ TEST_F(SubprocessTest, InterruptChildWithSigHup) {
   Subprocess* subproc = subprocs_.Add("kill -HUP $$");
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -138,7 +148,7 @@ TEST_F(SubprocessTest, InterruptParentWithSigHup) {
   ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
-    bool interrupted = subprocs_.DoWork();
+    bool interrupted = subprocs_.DoWork(NULL);
     if (interrupted)
       return;
   }
@@ -153,9 +163,11 @@ TEST_F(SubprocessTest, Console) {
         subprocs_.Add("test -t 0 -a -t 1 -a -t 2", /*use_console=*/true);
     ASSERT_NE((Subprocess*)0, subproc);
 
+    subprocs_.ResetTokenAvailable();
     while (!subproc->Done()) {
-      subprocs_.DoWork();
+      subprocs_.DoWork(NULL);
     }
+    ASSERT_EQ(false, subprocs_.IsTokenAvailable());
 
     EXPECT_EQ(ExitSuccess, subproc->Finish());
   }
@@ -167,9 +179,11 @@ TEST_F(SubprocessTest, SetWithSingle) {
   Subprocess* subproc = subprocs_.Add(kSimpleCommand);
   ASSERT_NE((Subprocess *) 0, subproc);
 
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
   ASSERT_EQ(ExitSuccess, subproc->Finish());
   ASSERT_NE("", subproc->GetOutput());
 
@@ -200,12 +214,13 @@ TEST_F(SubprocessTest, SetWithMulti) {
     ASSERT_EQ("", processes[i]->GetOutput());
   }
 
+  subprocs_.ResetTokenAvailable();
   while (!processes[0]->Done() || !processes[1]->Done() ||
          !processes[2]->Done()) {
     ASSERT_GT(subprocs_.running_.size(), 0u);
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
-
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
   ASSERT_EQ(0u, subprocs_.running_.size());
   ASSERT_EQ(3u, subprocs_.finished_.size());
 
@@ -237,8 +252,10 @@ TEST_F(SubprocessTest, SetWithLots) {
     ASSERT_NE((Subprocess *) 0, subproc);
     procs.push_back(subproc);
   }
+  subprocs_.ResetTokenAvailable();
   while (!subprocs_.running_.empty())
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
   for (size_t i = 0; i < procs.size(); ++i) {
     ASSERT_EQ(ExitSuccess, procs[i]->Finish());
     ASSERT_NE("", procs[i]->GetOutput());
@@ -254,9 +271,11 @@ TEST_F(SubprocessTest, SetWithLots) {
 // that stdin is closed.
 TEST_F(SubprocessTest, ReadStdin) {
   Subprocess* subproc = subprocs_.Add("cat -");
+  subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
-    subprocs_.DoWork();
+    subprocs_.DoWork(NULL);
   }
+  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
   ASSERT_EQ(ExitSuccess, subproc->Finish());
   ASSERT_EQ(1u, subprocs_.finished_.size());
 }
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index a8f9b7139d..396bb7d874 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -33,6 +33,7 @@ struct GNUmakeTokenPool : public TokenPool {
   virtual void Reserve();
   virtual void Release();
   virtual void Clear();
+  virtual int GetMonitorFd();
 
   bool Setup();
 
@@ -201,6 +202,10 @@ void GNUmakeTokenPool::Clear() {
     Return();
 }
 
+int GNUmakeTokenPool::GetMonitorFd() {
+  return(rfd_);
+}
+
 struct TokenPool *TokenPool::Get(void) {
   GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
   if (tokenpool->Setup())
diff --git a/src/tokenpool.h b/src/tokenpool.h
index f560b1083b..301e1998ee 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -21,6 +21,12 @@ struct TokenPool {
   virtual void Release() = 0;
   virtual void Clear() = 0;
 
+#ifdef _WIN32
+  // @TODO
+#else
+  virtual int GetMonitorFd() = 0;
+#endif
+
   // returns NULL if token pool is not available
   static struct TokenPool *Get(void);
 };

From d09f3d77821b3b1fdf09fc0ef8e814907675eafb Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Sun, 12 Nov 2017 16:58:55 +0200
Subject: [PATCH 03/11] Ignore jobserver when -jN is forced on command line

This emulates the behaviour of GNU make.

- add parallelism_from_cmdline flag to build configuration
- set the flag when -jN is given on command line
- pass the flag to TokenPool::Get()
- GNUmakeTokenPool::Setup()
  * prints a warning when the flag is true and jobserver was detected
  * returns false, i.e. jobserver will be ignored
- ignore config.parallelism in CanRunMore() when we have a valid
  TokenPool, because it gets always initialized to a default when not
  given on the command line
---
 src/build.cc              | 10 ++++++----
 src/build.h               |  4 +++-
 src/ninja.cc              |  1 +
 src/tokenpool-gnu-make.cc | 34 +++++++++++++++++++---------------
 src/tokenpool-none.cc     |  4 ++--
 src/tokenpool.h           |  4 ++--
 6 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/src/build.cc b/src/build.cc
index a25c349050..406a84ec39 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -470,7 +470,7 @@ struct RealCommandRunner : public CommandRunner {
 };
 
 RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
-  tokens_ = TokenPool::Get();
+  tokens_ = TokenPool::Get(config_.parallelism_from_cmdline);
 }
 
 RealCommandRunner::~RealCommandRunner() {
@@ -492,9 +492,11 @@ void RealCommandRunner::Abort() {
 }
 
 bool RealCommandRunner::CanRunMore() const {
-  size_t subproc_number =
-      subprocs_.running_.size() + subprocs_.finished_.size();
-  return (int)subproc_number < config_.parallelism
+  bool parallelism_limit_not_reached =
+    tokens_ || // ignore config_.parallelism
+    ((int) (subprocs_.running_.size() +
+            subprocs_.finished_.size()) < config_.parallelism);
+  return parallelism_limit_not_reached
     && (subprocs_.running_.empty() ||
         (config_.max_load_average <= 0.0f ||
          GetLoadAverage() < config_.max_load_average));
diff --git a/src/build.h b/src/build.h
index 35c7b97d12..dfde576573 100644
--- a/src/build.h
+++ b/src/build.h
@@ -159,7 +159,8 @@ struct CommandRunner {
 
 /// Options (e.g. verbosity, parallelism) passed to a build.
 struct BuildConfig {
-  BuildConfig() : verbosity(NORMAL), dry_run(false), parallelism(1),
+  BuildConfig() : verbosity(NORMAL), dry_run(false),
+                  parallelism(1), parallelism_from_cmdline(false),
                   failures_allowed(1), max_load_average(-0.0f) {}
 
   enum Verbosity {
@@ -171,6 +172,7 @@ struct BuildConfig {
   Verbosity verbosity;
   bool dry_run;
   int parallelism;
+  bool parallelism_from_cmdline;
   int failures_allowed;
   /// The maximum load average we must not exceed. A negative value
   /// means that we do not have any limit.
diff --git a/src/ninja.cc b/src/ninja.cc
index df39ba92d1..d904c56c4e 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -1447,6 +1447,7 @@ int ReadFlags(int* argc, char*** argv,
         // We want to run N jobs in parallel. For N = 0, INT_MAX
         // is close enough to infinite for most sane builds.
         config->parallelism = value > 0 ? value : INT_MAX;
+        config->parallelism_from_cmdline = true;
         deferGuessParallelism.needGuess = false;
         break;
       }
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index 396bb7d874..af4be05a31 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -1,4 +1,4 @@
-// Copyright 2016 Google Inc. All Rights Reserved.
+// Copyright 2016-2017 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ struct GNUmakeTokenPool : public TokenPool {
   virtual void Clear();
   virtual int GetMonitorFd();
 
-  bool Setup();
+  bool Setup(bool ignore);
 
  private:
   int available_;
@@ -100,7 +100,7 @@ bool GNUmakeTokenPool::SetAlarmHandler() {
   }
 }
 
-bool GNUmakeTokenPool::Setup() {
+bool GNUmakeTokenPool::Setup(bool ignore) {
   const char *value = getenv("MAKEFLAGS");
   if (value) {
     // GNU make <= 4.1
@@ -109,16 +109,20 @@ bool GNUmakeTokenPool::Setup() {
     if (!jobserver)
       jobserver = strstr(value, "--jobserver-auth=");
     if (jobserver) {
-      int rfd = -1;
-      int wfd = -1;
-      if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
-          CheckFd(rfd) &&
-          CheckFd(wfd) &&
-          SetAlarmHandler()) {
-        printf("ninja: using GNU make jobserver.\n");
-        rfd_ = rfd;
-        wfd_ = wfd;
-        return true;
+      if (ignore) {
+        printf("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
+      } else {
+        int rfd = -1;
+        int wfd = -1;
+        if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
+            CheckFd(rfd) &&
+            CheckFd(wfd) &&
+            SetAlarmHandler()) {
+          printf("ninja: using GNU make jobserver.\n");
+          rfd_ = rfd;
+          wfd_ = wfd;
+          return true;
+        }
       }
     }
   }
@@ -206,9 +210,9 @@ int GNUmakeTokenPool::GetMonitorFd() {
   return(rfd_);
 }
 
-struct TokenPool *TokenPool::Get(void) {
+struct TokenPool *TokenPool::Get(bool ignore) {
   GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
-  if (tokenpool->Setup())
+  if (tokenpool->Setup(ignore))
     return tokenpool;
   else
     delete tokenpool;
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
index 602b3316f5..199b22264b 100644
--- a/src/tokenpool-none.cc
+++ b/src/tokenpool-none.cc
@@ -1,4 +1,4 @@
-// Copyright 2016 Google Inc. All Rights Reserved.
+// Copyright 2016-2017 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,6 +22,6 @@
 #include <stdlib.h>
 
 // No-op TokenPool implementation
-struct TokenPool *TokenPool::Get(void) {
+struct TokenPool *TokenPool::Get(bool ignore) {
   return NULL;
 }
diff --git a/src/tokenpool.h b/src/tokenpool.h
index 301e1998ee..878a0933c2 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -1,4 +1,4 @@
-// Copyright 2016 Google Inc. All Rights Reserved.
+// Copyright 2016-2017 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -28,5 +28,5 @@ struct TokenPool {
 #endif
 
   // returns NULL if token pool is not available
-  static struct TokenPool *Get(void);
+  static struct TokenPool *Get(bool ignore);
 };

From dfe4ca753caee65bf9041e2b4e883dfa172a5c6a Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Sun, 12 Nov 2017 18:04:12 +0200
Subject: [PATCH 04/11] Honor -lN from MAKEFLAGS

This emulates the behaviour of GNU make.

- build: make a copy of max_load_average and pass it to TokenPool.
- GNUmakeTokenPool: if we detect a jobserver and a valid -lN argument in
  MAKEFLAGS then set max_load_average to N.
---
 src/build.cc              | 10 +++++++---
 src/tokenpool-gnu-make.cc | 19 +++++++++++++++----
 src/tokenpool-none.cc     |  2 +-
 src/tokenpool.h           |  2 +-
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/build.cc b/src/build.cc
index 406a84ec39..9e6272d035 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -464,13 +464,17 @@ struct RealCommandRunner : public CommandRunner {
   virtual void Abort();
 
   const BuildConfig& config_;
+  // copy of config_.max_load_average; can be modified by TokenPool setup
+  double max_load_average_;
   SubprocessSet subprocs_;
   TokenPool *tokens_;
   map<const Subprocess*, Edge*> subproc_to_edge_;
 };
 
 RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
-  tokens_ = TokenPool::Get(config_.parallelism_from_cmdline);
+  max_load_average_ = config.max_load_average;
+  tokens_ = TokenPool::Get(config_.parallelism_from_cmdline,
+                           max_load_average_);
 }
 
 RealCommandRunner::~RealCommandRunner() {
@@ -498,8 +502,8 @@ bool RealCommandRunner::CanRunMore() const {
             subprocs_.finished_.size()) < config_.parallelism);
   return parallelism_limit_not_reached
     && (subprocs_.running_.empty() ||
-        (config_.max_load_average <= 0.0f ||
-         GetLoadAverage() < config_.max_load_average));
+        (max_load_average_ <= 0.0f ||
+         GetLoadAverage() < max_load_average_));
 }
 
 bool RealCommandRunner::AcquireToken() {
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index af4be05a31..fb654c4d88 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -35,7 +35,7 @@ struct GNUmakeTokenPool : public TokenPool {
   virtual void Clear();
   virtual int GetMonitorFd();
 
-  bool Setup(bool ignore);
+  bool Setup(bool ignore, double& max_load_average);
 
  private:
   int available_;
@@ -100,7 +100,7 @@ bool GNUmakeTokenPool::SetAlarmHandler() {
   }
 }
 
-bool GNUmakeTokenPool::Setup(bool ignore) {
+bool GNUmakeTokenPool::Setup(bool ignore, double& max_load_average) {
   const char *value = getenv("MAKEFLAGS");
   if (value) {
     // GNU make <= 4.1
@@ -118,9 +118,20 @@ bool GNUmakeTokenPool::Setup(bool ignore) {
             CheckFd(rfd) &&
             CheckFd(wfd) &&
             SetAlarmHandler()) {
+          const char *l_arg = strstr(value, " -l");
+          int load_limit = -1;
+
           printf("ninja: using GNU make jobserver.\n");
           rfd_ = rfd;
           wfd_ = wfd;
+
+          // translate GNU make -lN to ninja -lN
+          if (l_arg &&
+              (sscanf(l_arg + 3, "%d ", &load_limit) == 1) &&
+              (load_limit > 0)) {
+            max_load_average = load_limit;
+          }
+
           return true;
         }
       }
@@ -210,9 +221,9 @@ int GNUmakeTokenPool::GetMonitorFd() {
   return(rfd_);
 }
 
-struct TokenPool *TokenPool::Get(bool ignore) {
+struct TokenPool *TokenPool::Get(bool ignore, double& max_load_average) {
   GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
-  if (tokenpool->Setup(ignore))
+  if (tokenpool->Setup(ignore, max_load_average))
     return tokenpool;
   else
     delete tokenpool;
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
index 199b22264b..e8e25426c3 100644
--- a/src/tokenpool-none.cc
+++ b/src/tokenpool-none.cc
@@ -22,6 +22,6 @@
 #include <stdlib.h>
 
 // No-op TokenPool implementation
-struct TokenPool *TokenPool::Get(bool ignore) {
+struct TokenPool *TokenPool::Get(bool ignore, double& max_load_average) {
   return NULL;
 }
diff --git a/src/tokenpool.h b/src/tokenpool.h
index 878a0933c2..f9e8cc2ee0 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -28,5 +28,5 @@ struct TokenPool {
 #endif
 
   // returns NULL if token pool is not available
-  static struct TokenPool *Get(bool ignore);
+  static struct TokenPool *Get(bool ignore, double& max_load_average);
 };

From 1c10047fc6a3269ba42839da19361e09cbc06ff0 Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Wed, 6 Dec 2017 22:14:21 +0200
Subject: [PATCH 05/11] Use LinePrinter for TokenPool messages

- replace printf() with calls to LinePrinter
- print GNU make jobserver message only when verbose build is requested
---
 src/build.cc              |  1 +
 src/tokenpool-gnu-make.cc | 22 ++++++++++++++++------
 src/tokenpool-none.cc     |  4 +++-
 src/tokenpool.h           |  4 +++-
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/build.cc b/src/build.cc
index 9e6272d035..662e4bd7be 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -474,6 +474,7 @@ struct RealCommandRunner : public CommandRunner {
 RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
   max_load_average_ = config.max_load_average;
   tokens_ = TokenPool::Get(config_.parallelism_from_cmdline,
+                           config_.verbosity == BuildConfig::VERBOSE,
                            max_load_average_);
 }
 
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index fb654c4d88..b0d3e6ebc4 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -23,6 +23,8 @@
 #include <string.h>
 #include <stdlib.h>
 
+#include "line_printer.h"
+
 // TokenPool implementation for GNU make jobserver
 // (http://make.mad-scientist.net/papers/jobserver-implementation/)
 struct GNUmakeTokenPool : public TokenPool {
@@ -35,7 +37,7 @@ struct GNUmakeTokenPool : public TokenPool {
   virtual void Clear();
   virtual int GetMonitorFd();
 
-  bool Setup(bool ignore, double& max_load_average);
+  bool Setup(bool ignore, bool verbose, double& max_load_average);
 
  private:
   int available_;
@@ -100,7 +102,9 @@ bool GNUmakeTokenPool::SetAlarmHandler() {
   }
 }
 
-bool GNUmakeTokenPool::Setup(bool ignore, double& max_load_average) {
+bool GNUmakeTokenPool::Setup(bool ignore,
+                             bool verbose,
+                             double& max_load_average) {
   const char *value = getenv("MAKEFLAGS");
   if (value) {
     // GNU make <= 4.1
@@ -109,8 +113,10 @@ bool GNUmakeTokenPool::Setup(bool ignore, double& max_load_average) {
     if (!jobserver)
       jobserver = strstr(value, "--jobserver-auth=");
     if (jobserver) {
+      LinePrinter printer;
+
       if (ignore) {
-        printf("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
+        printer.PrintOnNewLine("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
       } else {
         int rfd = -1;
         int wfd = -1;
@@ -121,7 +127,9 @@ bool GNUmakeTokenPool::Setup(bool ignore, double& max_load_average) {
           const char *l_arg = strstr(value, " -l");
           int load_limit = -1;
 
-          printf("ninja: using GNU make jobserver.\n");
+          if (verbose) {
+            printer.PrintOnNewLine("ninja: using GNU make jobserver.\n");
+          }
           rfd_ = rfd;
           wfd_ = wfd;
 
@@ -221,9 +229,11 @@ int GNUmakeTokenPool::GetMonitorFd() {
   return(rfd_);
 }
 
-struct TokenPool *TokenPool::Get(bool ignore, double& max_load_average) {
+struct TokenPool *TokenPool::Get(bool ignore,
+                                 bool verbose,
+                                 double& max_load_average) {
   GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
-  if (tokenpool->Setup(ignore, max_load_average))
+  if (tokenpool->Setup(ignore, verbose, max_load_average))
     return tokenpool;
   else
     delete tokenpool;
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
index e8e25426c3..1c1c499c8d 100644
--- a/src/tokenpool-none.cc
+++ b/src/tokenpool-none.cc
@@ -22,6 +22,8 @@
 #include <stdlib.h>
 
 // No-op TokenPool implementation
-struct TokenPool *TokenPool::Get(bool ignore, double& max_load_average) {
+struct TokenPool *TokenPool::Get(bool ignore,
+                                 bool verbose,
+                                 double& max_load_average) {
   return NULL;
 }
diff --git a/src/tokenpool.h b/src/tokenpool.h
index f9e8cc2ee0..4bf477f20c 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -28,5 +28,7 @@ struct TokenPool {
 #endif
 
   // returns NULL if token pool is not available
-  static struct TokenPool *Get(bool ignore, double& max_load_average);
+  static struct TokenPool *Get(bool ignore,
+                               bool verbose,
+                               double& max_load_average);
 };

From fdbf68416e3574add3bffd0b637d0694fbaba320 Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Sat, 7 Apr 2018 17:11:21 +0300
Subject: [PATCH 06/11] Prepare PR for merging

- fix Windows build error in no-op TokenPool implementation
- improve Acquire() to block for a maximum of 100ms
- address review comments
---
 src/build.h               |  2 ++
 src/tokenpool-gnu-make.cc | 53 +++++++++++++++++++++++++++++++++------
 src/tokenpool-none.cc     |  7 +-----
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/src/build.h b/src/build.h
index dfde576573..66ddefb888 100644
--- a/src/build.h
+++ b/src/build.h
@@ -151,6 +151,8 @@ struct CommandRunner {
     bool success() const { return status == ExitSuccess; }
   };
   /// Wait for a command to complete, or return false if interrupted.
+  /// If more_ready is true then the optional TokenPool is monitored too
+  /// and we return when a token becomes available.
   virtual bool WaitForCommand(Result* result, bool more_ready) = 0;
 
   virtual std::vector<Edge*> GetActiveEdges() { return std::vector<Edge*>(); }
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index b0d3e6ebc4..4132bb06d9 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -1,4 +1,4 @@
-// Copyright 2016-2017 Google Inc. All Rights Reserved.
+// Copyright 2016-2018 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
 #include <poll.h>
 #include <unistd.h>
 #include <signal.h>
+#include <sys/time.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -153,6 +154,15 @@ bool GNUmakeTokenPool::Acquire() {
   if (available_ > 0)
     return true;
 
+  // Please read
+  //
+  //   http://make.mad-scientist.net/papers/jobserver-implementation/
+  //
+  // for the reasoning behind the following code.
+  //
+  // Try to read one character from the pipe. Returns true on success.
+  //
+  // First check if read() would succeed without blocking.
 #ifdef USE_PPOLL
   pollfd pollfds[] = {{rfd_, POLLIN, 0}};
   int ret = poll(pollfds, 1, 0);
@@ -164,33 +174,62 @@ bool GNUmakeTokenPool::Acquire() {
   int ret = select(rfd_ + 1, &set, NULL, NULL, &timeout);
 #endif
   if (ret > 0) {
+    // Handle potential race condition:
+    //  - the above check succeeded, i.e. read() should not block
+    //  - the character disappears before we call read()
+    //
+    // Create a duplicate of rfd_. The duplicate file descriptor dup_rfd_
+    // can safely be closed by signal handlers without affecting rfd_.
     dup_rfd_ = dup(rfd_);
 
     if (dup_rfd_ != -1) {
       struct sigaction act, old_act;
       int ret = 0;
 
+      // Temporarily replace SIGCHLD handler with our own
       memset(&act, 0, sizeof(act));
       act.sa_handler = CloseDupRfd;
       if (sigaction(SIGCHLD, &act, &old_act) == 0) {
-        char buf;
-
-        // block until token read, child exits or timeout
-        alarm(1);
-        ret = read(dup_rfd_, &buf, 1);
-        alarm(0);
+        struct itimerval timeout;
+
+        // install a 100ms timeout that generates SIGALARM on expiration
+        memset(&timeout, 0, sizeof(timeout));
+        timeout.it_value.tv_usec = 100 * 1000; // [ms] -> [usec]
+        if (setitimer(ITIMER_REAL, &timeout, NULL) == 0) {
+          char buf;
+
+          // Now try to read() from dup_rfd_. Return values from read():
+          //
+          // 1. token read                               ->  1
+          // 2. pipe closed                              ->  0
+          // 3. alarm expires                            -> -1 (EINTR)
+          // 4. child exits                              -> -1 (EINTR)
+          // 5. alarm expired before entering read()     -> -1 (EBADF)
+          // 6. child exited before entering read()      -> -1 (EBADF)
+          // 7. child exited before handler is installed -> go to 1 - 3
+          ret = read(dup_rfd_, &buf, 1);
+
+          // disarm timer
+          memset(&timeout, 0, sizeof(timeout));
+          setitimer(ITIMER_REAL, &timeout, NULL);
+        }
 
         sigaction(SIGCHLD, &old_act, NULL);
       }
 
       CloseDupRfd(0);
 
+      // Case 1 from above list
       if (ret > 0) {
         available_++;
         return true;
       }
     }
   }
+
+  // read() would block, i.e. no token available,
+  // cases 2-6 from above list or
+  // select() / poll() / dup() / sigaction() / setitimer() failed
   return false;
 }
 
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
index 1c1c499c8d..4c592875b4 100644
--- a/src/tokenpool-none.cc
+++ b/src/tokenpool-none.cc
@@ -1,4 +1,4 @@
-// Copyright 2016-2017 Google Inc. All Rights Reserved.
+// Copyright 2016-2018 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,11 +14,6 @@
 
 #include "tokenpool.h"
 
-#include <fcntl.h>
-#include <poll.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <string.h>
 #include <stdlib.h>
 
 // No-op TokenPool implementation

From ec6220a0baf7d3a6eaf1a2b75bf8960ddfe24c2f Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Fri, 25 May 2018 00:17:07 +0300
Subject: [PATCH 07/11] Add tests for TokenPool

- TokenPool setup
- GetMonitorFd() API
- implicit token and tokens in jobserver pipe
- Acquire() / Reserve() / Release() protocol
- Clear() API
---
 configure.py          |   1 +
 src/tokenpool_test.cc | 198 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 199 insertions(+)
 create mode 100644 src/tokenpool_test.cc

diff --git a/configure.py b/configure.py
index db3492c93c..dc8a0066b7 100755
--- a/configure.py
+++ b/configure.py
@@ -590,6 +590,7 @@ def has_re2c():
              'string_piece_util_test',
              'subprocess_test',
              'test',
+             'tokenpool_test',
              'util_test']:
     objs += cxx(name, variables=cxxvariables)
 if platform.is_windows():
diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
new file mode 100644
index 0000000000..6c89064ca4
--- /dev/null
+++ b/src/tokenpool_test.cc
@@ -0,0 +1,198 @@
+// Copyright 2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool.h"
+
+#include "test.h"
+
+#ifndef _WIN32
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
+#define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true);
+#endif
+
+namespace {
+
+const double kLoadAverageDefault = -1.23456789;
+
+struct TokenPoolTest : public testing::Test {
+  double load_avg_;
+  TokenPool *tokens_;
+#ifndef _WIN32
+  char buf_[1024];
+  int fds_[2];
+#endif
+
+  virtual void SetUp() {
+    load_avg_ = kLoadAverageDefault;
+    tokens_ = NULL;
+#ifndef _WIN32
+    ENVIRONMENT_CLEAR();
+    if (pipe(fds_) < 0)
+      ASSERT_TRUE(false);
+#endif
+  }
+
+  void CreatePool(const char *format, bool ignore_jobserver) {
+#ifndef _WIN32
+    if (format) {
+      sprintf(buf_, format, fds_[0], fds_[1]);
+      ENVIRONMENT_INIT(buf_);
+    }
+#endif
+    tokens_ = TokenPool::Get(ignore_jobserver, false, load_avg_);
+  }
+
+  void CreateDefaultPool() {
+    CreatePool("foo --jobserver-auth=%d,%d bar", false);
+  }
+
+  virtual void TearDown() {
+    if (tokens_)
+      delete tokens_;
+#ifndef _WIN32
+    close(fds_[0]);
+    close(fds_[1]);
+    ENVIRONMENT_CLEAR();
+#endif
+  }
+};
+
+} // anonymous namespace
+
+// verifies none implementation
+TEST_F(TokenPoolTest, NoTokenPool) {
+  CreatePool(NULL, false);
+
+  EXPECT_EQ(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+}
+
+#ifndef _WIN32
+TEST_F(TokenPoolTest, SuccessfulOldSetup) {
+  // GNUmake <= 4.1
+  CreatePool("foo --jobserver-fds=%d,%d bar", false);
+
+  EXPECT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+}
+
+TEST_F(TokenPoolTest, SuccessfulNewSetup) {
+  // GNUmake => 4.2
+  CreateDefaultPool();
+
+  EXPECT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+}
+
+TEST_F(TokenPoolTest, IgnoreWithJN) {
+  CreatePool("foo --jobserver-auth=%d,%d bar", true);
+
+  EXPECT_EQ(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+}
+
+TEST_F(TokenPoolTest, HonorLN) {
+  CreatePool("foo -l9 --jobserver-auth=%d,%d bar", false);
+
+  EXPECT_NE(NULL, tokens_);
+  EXPECT_EQ(9.0, load_avg_);
+}
+
+TEST_F(TokenPoolTest, MonitorFD) {
+  CreateDefaultPool();
+
+  ASSERT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+
+  EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
+}
+
+TEST_F(TokenPoolTest, ImplicitToken) {
+  CreateDefaultPool();
+
+  ASSERT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_FALSE(tokens_->Acquire());
+  tokens_->Release();
+  EXPECT_TRUE(tokens_->Acquire());
+}
+
+TEST_F(TokenPoolTest, TwoTokens) {
+  CreateDefaultPool();
+
+  ASSERT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+
+  // implicit token
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_FALSE(tokens_->Acquire());
+
+  // jobserver offers 2nd token
+  ASSERT_EQ(1u, write(fds_[1], "T", 1));
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_FALSE(tokens_->Acquire());
+
+  // release 2nd token
+  tokens_->Release();
+  EXPECT_TRUE(tokens_->Acquire());
+
+  // release implict token - must return 2nd token back to jobserver
+  tokens_->Release();
+  EXPECT_TRUE(tokens_->Acquire());
+
+  // there must be one token in the pipe
+  EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
+
+  // implicit token
+  EXPECT_TRUE(tokens_->Acquire());
+}
+
+TEST_F(TokenPoolTest, Clear) {
+  CreateDefaultPool();
+
+  ASSERT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+
+  // implicit token
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_FALSE(tokens_->Acquire());
+
+  // jobserver offers 2nd & 3rd token
+  ASSERT_EQ(2u, write(fds_[1], "TT", 2));
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_TRUE(tokens_->Acquire());
+  tokens_->Reserve();
+  EXPECT_FALSE(tokens_->Acquire());
+
+  tokens_->Clear();
+  EXPECT_TRUE(tokens_->Acquire());
+
+  // there must be two tokens in the pipe
+  EXPECT_EQ(2u, read(fds_[0], buf_, sizeof(buf_)));
+
+  // implicit token
+  EXPECT_TRUE(tokens_->Acquire());
+}
+#endif

From e59d8858327126d1593fd0b8e607975a79072e92 Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Thu, 24 May 2018 18:52:45 +0300
Subject: [PATCH 08/11] Add tests for subprocess module

- add TokenPoolTest stub to provide TokenPool::GetMonitorFd()
- add two tests
  * both tests set up a dummy GNUmake jobserver pipe
  * both tests call DoWork() with TokenPoolTest
  * test 1: verify that DoWork() detects when a token is available
  * test 2: verify that DoWork() works as before without a token
- the tests are not compiled in under Windows
---
 src/subprocess_test.cc | 76 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
index 4bc8083e26..6264c8bf11 100644
--- a/src/subprocess_test.cc
+++ b/src/subprocess_test.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "subprocess.h"
+#include "tokenpool.h"
 
 #include "test.h"
 
@@ -34,8 +35,23 @@ const char* kSimpleCommand = "cmd /c dir \\";
 const char* kSimpleCommand = "ls /";
 #endif
 
+struct TokenPoolTest : public TokenPool {
+  bool Acquire()     { return false; }
+  void Reserve()     {}
+  void Release()     {}
+  void Clear()       {}
+
+#ifdef _WIN32
+  // @TODO
+#else
+  int _fd;
+  int GetMonitorFd() { return _fd; }
+#endif
+};
+
 struct SubprocessTest : public testing::Test {
   SubprocessSet subprocs_;
+  TokenPoolTest tokens_;
 };
 
 }  // anonymous namespace
@@ -280,3 +296,63 @@ TEST_F(SubprocessTest, ReadStdin) {
   ASSERT_EQ(1u, subprocs_.finished_.size());
 }
 #endif  // _WIN32
+
+// @TODO: remove once TokenPool implementation for Windows is available
+#ifndef _WIN32
+TEST_F(SubprocessTest, TokenAvailable) {
+  Subprocess* subproc = subprocs_.Add(kSimpleCommand);
+  ASSERT_NE((Subprocess *) 0, subproc);
+
+  // simulate GNUmake jobserver pipe with 1 token
+  int fds[2];
+  ASSERT_EQ(0u, pipe(fds));
+  tokens_._fd = fds[0];
+  ASSERT_EQ(1u, write(fds[1], "T", 1));
+
+  subprocs_.ResetTokenAvailable();
+  subprocs_.DoWork(&tokens_);
+
+  EXPECT_TRUE(subprocs_.IsTokenAvailable());
+  EXPECT_EQ(0u, subprocs_.finished_.size());
+
+  // remove token to let DoWork() wait for command again
+  char token;
+  ASSERT_EQ(1u, read(fds[0], &token, 1));
+
+  while (!subproc->Done()) {
+    subprocs_.DoWork(&tokens_);
+  }
+
+  close(fds[1]);
+  close(fds[0]);
+
+  EXPECT_EQ(ExitSuccess, subproc->Finish());
+  EXPECT_NE("", subproc->GetOutput());
+
+  EXPECT_EQ(1u, subprocs_.finished_.size());
+}
+
+TEST_F(SubprocessTest, TokenNotAvailable) {
+  Subprocess* subproc = subprocs_.Add(kSimpleCommand);
+  ASSERT_NE((Subprocess *) 0, subproc);
+
+  // simulate GNUmake jobserver pipe with 0 tokens
+  int fds[2];
+  ASSERT_EQ(0u, pipe(fds));
+  tokens_._fd = fds[0];
+
+  subprocs_.ResetTokenAvailable();
+  while (!subproc->Done()) {
+    subprocs_.DoWork(&tokens_);
+  }
+
+  close(fds[1]);
+  close(fds[0]);
+
+  EXPECT_FALSE(subprocs_.IsTokenAvailable());
+  EXPECT_EQ(ExitSuccess, subproc->Finish());
+  EXPECT_NE("", subproc->GetOutput());
+
+  EXPECT_EQ(1u, subprocs_.finished_.size());
+}
+#endif  // _WIN32

From 0145e2d4db64ea6c21aeb371928e4071f65164eb Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Sat, 26 May 2018 23:17:51 +0300
Subject: [PATCH 09/11] Add tests for build module

Add tests that verify the token functionality of the builder main loop.
We replace the default fake command runner with a special version where
the tests can control each call to AcquireToken(), CanRunMore() and
WaitForCommand().
---
 src/build_test.cc | 364 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 364 insertions(+)

diff --git a/src/build_test.cc b/src/build_test.cc
index 7a5ff4015a..dd41dfbe1d 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -15,6 +15,7 @@
 #include "build.h"
 
 #include <assert.h>
+#include <stdarg.h>
 
 #include "build_log.h"
 #include "deps_log.h"
@@ -3990,3 +3991,366 @@ TEST_F(BuildTest, ValidationWithCircularDependency) {
   EXPECT_FALSE(builder_.AddTarget("out", &err));
   EXPECT_EQ("dependency cycle: validate -> validate_in -> validate", err);
 }
+
+/// The token tests are concerned with the main loop functionality when
+// the CommandRunner has an active TokenPool. It is therefore intentional
+// that the plan doesn't complete and that builder_.Build() returns false!
+
+/// Fake implementation of CommandRunner that simulates a TokenPool
+struct FakeTokenCommandRunner : public CommandRunner {
+  explicit FakeTokenCommandRunner() {}
+
+  // CommandRunner impl
+  virtual bool CanRunMore() const;
+  virtual bool AcquireToken();
+  virtual bool StartCommand(Edge* edge);
+  virtual bool WaitForCommand(Result* result, bool more_ready);
+  virtual vector<Edge*> GetActiveEdges();
+  virtual void Abort();
+
+  vector<string> commands_ran_;
+  vector<Edge *> edges_;
+
+  vector<bool> acquire_token_;
+  vector<bool> can_run_more_;
+  vector<bool> wait_for_command_;
+};
+
+bool FakeTokenCommandRunner::CanRunMore() const {
+  if (can_run_more_.size() == 0) {
+    EXPECT_FALSE("unexpected call to CommandRunner::CanRunMore()");
+    return false;
+  }
+
+  bool result = can_run_more_[0];
+
+  // Unfortunately CanRunMore() isn't "const" for tests
+  const_cast<FakeTokenCommandRunner*>(this)->can_run_more_.erase(
+    const_cast<FakeTokenCommandRunner*>(this)->can_run_more_.begin()
+  );
+
+  return result;
+}
+
+bool FakeTokenCommandRunner::AcquireToken() {
+  if (acquire_token_.size() == 0) {
+    EXPECT_FALSE("unexpected call to CommandRunner::AcquireToken()");
+    return false;
+  }
+
+  bool result = acquire_token_[0];
+  acquire_token_.erase(acquire_token_.begin());
+  return result;
+}
+
+bool FakeTokenCommandRunner::StartCommand(Edge* edge) {
+  commands_ran_.push_back(edge->EvaluateCommand());
+  edges_.push_back(edge);
+  return true;
+}
+
+bool FakeTokenCommandRunner::WaitForCommand(Result* result, bool more_ready) {
+  if (wait_for_command_.size() == 0) {
+    EXPECT_FALSE("unexpected call to CommandRunner::WaitForCommand()");
+    return false;
+  }
+
+  bool expected = wait_for_command_[0];
+  if (expected != more_ready) {
+    EXPECT_EQ(expected, more_ready);
+    return false;
+  }
+  wait_for_command_.erase(wait_for_command_.begin());
+
+  if (edges_.size() == 0)
+    return false;
+
+  Edge* edge = edges_[0];
+  result->edge = edge;
+
+  if (more_ready &&
+      (edge->rule().name() == "token-available")) {
+    result->status = ExitTokenAvailable;
+  } else {
+    edges_.erase(edges_.begin());
+    result->status = ExitSuccess;
+  }
+
+  return true;
+}
+
+vector<Edge*> FakeTokenCommandRunner::GetActiveEdges() {
+  return edges_;
+}
+
+void FakeTokenCommandRunner::Abort() {
+  edges_.clear();
+}
+
+struct BuildTokenTest : public BuildTest {
+  virtual void SetUp();
+  virtual void TearDown();
+
+  FakeTokenCommandRunner token_command_runner_;
+
+  void ExpectAcquireToken(int count, ...);
+  void ExpectCanRunMore(int count, ...);
+  void ExpectWaitForCommand(int count, ...);
+
+private:
+  void EnqueueBooleans(vector<bool>& booleans, int count, va_list ao);
+};
+
+void BuildTokenTest::SetUp() {
+  BuildTest::SetUp();
+
+  // replace FakeCommandRunner with FakeTokenCommandRunner
+  builder_.command_runner_.release();
+  builder_.command_runner_.reset(&token_command_runner_);
+}
+void BuildTokenTest::TearDown() {
+  EXPECT_EQ(0u, token_command_runner_.acquire_token_.size());
+  EXPECT_EQ(0u, token_command_runner_.can_run_more_.size());
+  EXPECT_EQ(0u, token_command_runner_.wait_for_command_.size());
+
+  BuildTest::TearDown();
+}
+
+void BuildTokenTest::ExpectAcquireToken(int count, ...) {
+  va_list ap;
+  va_start(ap, count);
+  EnqueueBooleans(token_command_runner_.acquire_token_, count, ap);
+  va_end(ap);
+}
+
+void BuildTokenTest::ExpectCanRunMore(int count, ...) {
+  va_list ap;
+  va_start(ap, count);
+  EnqueueBooleans(token_command_runner_.can_run_more_, count, ap);
+  va_end(ap);
+}
+
+void BuildTokenTest::ExpectWaitForCommand(int count, ...) {
+  va_list ap;
+  va_start(ap, count);
+  EnqueueBooleans(token_command_runner_.wait_for_command_, count, ap);
+  va_end(ap);
+}
+
+void BuildTokenTest::EnqueueBooleans(vector<bool>& booleans, int count, va_list ap) {
+  while (count--) {
+    int value = va_arg(ap, int);
+    booleans.push_back(!!value); // force bool
+  }
+}
+
+TEST_F(BuildTokenTest, CompleteNoWork) {
+  // plan should not execute anything
+  string err;
+
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(0u, token_command_runner_.commands_ran_.size());
+}
+
+TEST_F(BuildTokenTest, DoNotAquireToken) {
+  // plan should execute one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat1", &err));
+  ASSERT_EQ("", err);
+
+  // pretend we can't run anything
+  ExpectCanRunMore(1, false);
+
+  EXPECT_FALSE(builder_.Build(&err));
+  EXPECT_EQ("stuck [this is a bug]", err);
+
+  EXPECT_EQ(0u, token_command_runner_.commands_ran_.size());
+}
+
+TEST_F(BuildTokenTest, DoNotStartWithoutToken) {
+  // plan should execute one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat1", &err));
+  ASSERT_EQ("", err);
+
+  // we could run a command but do not have a token for it
+  ExpectCanRunMore(1,   true);
+  ExpectAcquireToken(1, false);
+
+  EXPECT_FALSE(builder_.Build(&err));
+  EXPECT_EQ("stuck [this is a bug]", err);
+
+  EXPECT_EQ(0u, token_command_runner_.commands_ran_.size());
+}
+
+TEST_F(BuildTokenTest, CompleteOneStep) {
+  // plan should execute one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat1", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of one command
+  ExpectCanRunMore(1,   true);
+  ExpectAcquireToken(1, true);
+  // block and wait for command to finalize
+  ExpectWaitForCommand(1, false);
+
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(1u, token_command_runner_.commands_ran_.size());
+  EXPECT_TRUE(token_command_runner_.commands_ran_[0] == "cat in1 > cat1");
+}
+
+TEST_F(BuildTokenTest, AcquireOneToken) {
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat12", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of one command
+  ExpectCanRunMore(3,     true, false, false);
+  ExpectAcquireToken(1,   true);
+  // block and wait for command to finalize
+  ExpectWaitForCommand(1, false);
+
+  EXPECT_FALSE(builder_.Build(&err));
+  EXPECT_EQ("stuck [this is a bug]", err);
+
+  EXPECT_EQ(1u, token_command_runner_.commands_ran_.size());
+  // any of the two dependencies could have been executed
+  EXPECT_TRUE(token_command_runner_.commands_ran_[0] == "cat in1 > cat1" ||
+              token_command_runner_.commands_ran_[0] == "cat in1 in2 > cat2");
+}
+
+TEST_F(BuildTokenTest, WantTwoTokens) {
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat12", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of one command
+  ExpectCanRunMore(3,     true, true, false);
+  ExpectAcquireToken(2,   true, false);
+  // wait for command to finalize or token to become available
+  ExpectWaitForCommand(1, true);
+
+  EXPECT_FALSE(builder_.Build(&err));
+  EXPECT_EQ("stuck [this is a bug]", err);
+
+  EXPECT_EQ(1u, token_command_runner_.commands_ran_.size());
+  // any of the two dependencies could have been executed
+  EXPECT_TRUE(token_command_runner_.commands_ran_[0] == "cat in1 > cat1" ||
+              token_command_runner_.commands_ran_[0] == "cat in1 in2 > cat2");
+}
+
+TEST_F(BuildTokenTest, CompleteTwoSteps) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build out1: cat in1\n"
+"build out2: cat out1\n"));
+
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out2", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of two commands
+  ExpectCanRunMore(2,     true, true);
+  ExpectAcquireToken(2,   true, true);
+  // wait for commands to finalize
+  ExpectWaitForCommand(2, false, false);
+
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(2u, token_command_runner_.commands_ran_.size());
+  EXPECT_TRUE(token_command_runner_.commands_ran_[0] == "cat in1 > out1");
+  EXPECT_TRUE(token_command_runner_.commands_ran_[1] == "cat out1 > out2");
+}
+
+TEST_F(BuildTokenTest, TwoCommandsInParallel) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule token-available\n"
+"  command = cat $in > $out\n"
+"build out1: token-available in1\n"
+"build out2: token-available in2\n"
+"build out12: cat out1 out2\n"));
+
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out12", &err));
+  ASSERT_EQ("", err);
+
+  // 1st command: token available -> allow running
+  // 2nd command: no token available but becomes available later
+  ExpectCanRunMore(4,     true, true,  true,  false);
+  ExpectAcquireToken(3,   true, false, true);
+  // 1st call waits for command to finalize or token to become available
+  // 2nd call waits for command to finalize
+  // 3rd call waits for command to finalize
+  ExpectWaitForCommand(3, true, false, false);
+
+  EXPECT_FALSE(builder_.Build(&err));
+  EXPECT_EQ("stuck [this is a bug]", err);
+
+  EXPECT_EQ(2u, token_command_runner_.commands_ran_.size());
+  EXPECT_TRUE((token_command_runner_.commands_ran_[0] == "cat in1 > out1" &&
+               token_command_runner_.commands_ran_[1] == "cat in2 > out2") ||
+              (token_command_runner_.commands_ran_[0] == "cat in2 > out2" &&
+               token_command_runner_.commands_ran_[1] == "cat in1 > out1"));
+}
+
+TEST_F(BuildTokenTest, CompleteThreeStepsSerial) {
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("cat12", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of all commands
+  ExpectCanRunMore(4,     true, true,  true,  true);
+  ExpectAcquireToken(4,   true, false, true,  true);
+  // wait for commands to finalize
+  ExpectWaitForCommand(3, true, false, false);
+
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(3u, token_command_runner_.commands_ran_.size());
+  EXPECT_TRUE((token_command_runner_.commands_ran_[0] == "cat in1 > cat1"     &&
+               token_command_runner_.commands_ran_[1] == "cat in1 in2 > cat2") ||
+              (token_command_runner_.commands_ran_[0] == "cat in1 in2 > cat2" &&
+               token_command_runner_.commands_ran_[1] == "cat in1 > cat1"    ));
+  EXPECT_TRUE(token_command_runner_.commands_ran_[2] == "cat cat1 cat2 > cat12");
+}
+
+TEST_F(BuildTokenTest, CompleteThreeStepsParallel) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule token-available\n"
+"  command = cat $in > $out\n"
+"build out1: token-available in1\n"
+"build out2: token-available in2\n"
+"build out12: cat out1 out2\n"));
+
+  // plan should execute more than one command
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out12", &err));
+  ASSERT_EQ("", err);
+
+  // allow running of all commands
+  ExpectCanRunMore(4,     true, true,  true,  true);
+  ExpectAcquireToken(4,   true, false, true,  true);
+  // wait for commands to finalize
+  ExpectWaitForCommand(4, true, false, false, false);
+
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(3u, token_command_runner_.commands_ran_.size());
+  EXPECT_TRUE((token_command_runner_.commands_ran_[0] == "cat in1 > out1" &&
+               token_command_runner_.commands_ran_[1] == "cat in2 > out2") ||
+              (token_command_runner_.commands_ran_[0] == "cat in2 > out2" &&
+               token_command_runner_.commands_ran_[1] == "cat in1 > out1"));
+  EXPECT_TRUE(token_command_runner_.commands_ran_[2] == "cat out1 out2 > out12");
+}

From f016e5430c9123d34a73ea7ad28693b20ee59d6d Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Mon, 8 Oct 2018 17:47:50 +0300
Subject: [PATCH 10/11] Add Win32 implementation for GNUmakeTokenPool

GNU make uses a semaphore as jobserver protocol on Win32. See also

   https://www.gnu.org/software/make/manual/html_node/Windows-Jobserver.html

Usage is pretty simple and straightforward, i.e. WaitForSingleObject()
to obtain a token and ReleaseSemaphore() to return it.

Unfortunately subprocess-win32.cc uses an I/O completion port (IOCP).
IOCPs aren't waitable objects, i.e. we can't use WaitForMultipleObjects()
to wait on the IOCP and the token semaphore at the same time.

Therefore GNUmakeTokenPoolWin32 creates a child thread that waits on the
token semaphore and posts a dummy I/O completion status on the IOCP when
it was able to obtain a token. That unblocks SubprocessSet::DoWork() and
it can then check if a token became available or not.

- split existing GNUmakeTokenPool into common and platform bits
- add GNUmakeTokenPool interface
- move the Posix bits to GNUmakeTokenPoolPosix
- add the Win32 bits as GNUmakeTokenPoolWin32
- move Setup() method up to TokenPool interface
- update Subprocess & TokenPool tests accordingly
---
 configure.py                    |   8 +-
 src/build.cc                    |  11 +-
 src/subprocess-win32.cc         |   9 ++
 src/subprocess_test.cc          |  34 ++++-
 src/tokenpool-gnu-make-posix.cc | 203 +++++++++++++++++++++++++++
 src/tokenpool-gnu-make-win32.cc | 237 ++++++++++++++++++++++++++++++++
 src/tokenpool-gnu-make.cc       | 203 ++-------------------------
 src/tokenpool-gnu-make.h        |  40 ++++++
 src/tokenpool-none.cc           |   4 +-
 src/tokenpool.h                 |  18 ++-
 src/tokenpool_test.cc           | 113 ++++++++++++---
 11 files changed, 653 insertions(+), 227 deletions(-)
 create mode 100644 src/tokenpool-gnu-make-posix.cc
 create mode 100644 src/tokenpool-gnu-make-win32.cc
 create mode 100644 src/tokenpool-gnu-make.h

diff --git a/configure.py b/configure.py
index dc8a0066b7..a239b90eef 100755
--- a/configure.py
+++ b/configure.py
@@ -517,12 +517,13 @@ def has_re2c():
              'state',
              'status',
              'string_piece_util',
+             'tokenpool-gnu-make',
              'util',
              'version']:
     objs += cxx(name, variables=cxxvariables)
 if platform.is_windows():
     for name in ['subprocess-win32',
-                 'tokenpool-none',
+                 'tokenpool-gnu-make-win32',
                  'includes_normalize-win32',
                  'msvc_helper-win32',
                  'msvc_helper_main-win32']:
@@ -531,8 +532,9 @@ def has_re2c():
         objs += cxx('minidump-win32', variables=cxxvariables)
     objs += cc('getopt')
 else:
-    objs += cxx('subprocess-posix')
-    objs += cxx('tokenpool-gnu-make')
+    for name in ['subprocess-posix',
+                 'tokenpool-gnu-make-posix']:
+        objs += cxx(name)
 if platform.is_aix():
     objs += cc('getopt')
 if platform.is_msvc():
diff --git a/src/build.cc b/src/build.cc
index 662e4bd7be..20c3bdc2a0 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -473,9 +473,14 @@ struct RealCommandRunner : public CommandRunner {
 
 RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
   max_load_average_ = config.max_load_average;
-  tokens_ = TokenPool::Get(config_.parallelism_from_cmdline,
-                           config_.verbosity == BuildConfig::VERBOSE,
-                           max_load_average_);
+  if ((tokens_ = TokenPool::Get()) != NULL) {
+    if (!tokens_->Setup(config_.parallelism_from_cmdline,
+                        config_.verbosity == BuildConfig::VERBOSE,
+                        max_load_average_)) {
+      delete tokens_;
+      tokens_ = NULL;
+    }
+  }
 }
 
 RealCommandRunner::~RealCommandRunner() {
diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc
index 66d2c2c430..ce3e2c20a4 100644
--- a/src/subprocess-win32.cc
+++ b/src/subprocess-win32.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "subprocess.h"
+#include "tokenpool.h"
 
 #include <assert.h>
 #include <stdio.h>
@@ -256,6 +257,9 @@ bool SubprocessSet::DoWork(struct TokenPool* tokens) {
   Subprocess* subproc;
   OVERLAPPED* overlapped;
 
+  if (tokens)
+    tokens->WaitForTokenAvailability(ioport_);
+
   if (!GetQueuedCompletionStatus(ioport_, &bytes_read, (PULONG_PTR)&subproc,
                                  &overlapped, INFINITE)) {
     if (GetLastError() != ERROR_BROKEN_PIPE)
@@ -266,6 +270,11 @@ bool SubprocessSet::DoWork(struct TokenPool* tokens) {
                 // delivered by NotifyInterrupted above.
     return true;
 
+  if (tokens && tokens->TokenIsAvailable((ULONG_PTR)subproc)) {
+    token_available_ = true;
+    return false;
+  }
+
   subproc->OnPipeReady();
 
   if (subproc->Done()) {
diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
index 6264c8bf11..f625963462 100644
--- a/src/subprocess_test.cc
+++ b/src/subprocess_test.cc
@@ -40,9 +40,16 @@ struct TokenPoolTest : public TokenPool {
   void Reserve()     {}
   void Release()     {}
   void Clear()       {}
+  bool Setup(bool ignore_unused, bool verbose, double& max_load_average) { return false; }
 
 #ifdef _WIN32
-  // @TODO
+  bool _token_available;
+  void WaitForTokenAvailability(HANDLE ioport) {
+    if (_token_available)
+      // unblock GetQueuedCompletionStatus()
+      PostQueuedCompletionStatus(ioport, 0, (ULONG_PTR) this, NULL);
+  }
+  bool TokenIsAvailable(ULONG_PTR key) { return key == (ULONG_PTR) this; }
 #else
   int _fd;
   int GetMonitorFd() { return _fd; }
@@ -297,34 +304,48 @@ TEST_F(SubprocessTest, ReadStdin) {
 }
 #endif  // _WIN32
 
-// @TODO: remove once TokenPool implementation for Windows is available
-#ifndef _WIN32
 TEST_F(SubprocessTest, TokenAvailable) {
   Subprocess* subproc = subprocs_.Add(kSimpleCommand);
   ASSERT_NE((Subprocess *) 0, subproc);
 
   // simulate GNUmake jobserver pipe with 1 token
+#ifdef _WIN32
+  tokens_._token_available = true;
+#else
   int fds[2];
   ASSERT_EQ(0u, pipe(fds));
   tokens_._fd = fds[0];
   ASSERT_EQ(1u, write(fds[1], "T", 1));
+#endif
 
   subprocs_.ResetTokenAvailable();
   subprocs_.DoWork(&tokens_);
+#ifdef _WIN32
+  tokens_._token_available = false;
+  // we need to loop here as we have no conrol where the token
+  // I/O completion post ends up in the queue
+  while (!subproc->Done() && !subprocs_.IsTokenAvailable()) {
+    subprocs_.DoWork(&tokens_);
+  }
+#endif
 
   EXPECT_TRUE(subprocs_.IsTokenAvailable());
   EXPECT_EQ(0u, subprocs_.finished_.size());
 
   // remove token to let DoWork() wait for command again
+#ifndef _WIN32
   char token;
   ASSERT_EQ(1u, read(fds[0], &token, 1));
+#endif
 
   while (!subproc->Done()) {
     subprocs_.DoWork(&tokens_);
   }
 
+#ifndef _WIN32
   close(fds[1]);
   close(fds[0]);
+#endif
 
   EXPECT_EQ(ExitSuccess, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
@@ -337,17 +358,23 @@ TEST_F(SubprocessTest, TokenNotAvailable) {
   ASSERT_NE((Subprocess *) 0, subproc);
 
   // simulate GNUmake jobserver pipe with 0 tokens
+#ifdef _WIN32
+  tokens_._token_available = false;
+#else
   int fds[2];
   ASSERT_EQ(0u, pipe(fds));
   tokens_._fd = fds[0];
+#endif
 
   subprocs_.ResetTokenAvailable();
   while (!subproc->Done()) {
     subprocs_.DoWork(&tokens_);
   }
 
+#ifndef _WIN32
   close(fds[1]);
   close(fds[0]);
+#endif
 
   EXPECT_FALSE(subprocs_.IsTokenAvailable());
   EXPECT_EQ(ExitSuccess, subproc->Finish());
@@ -355,4 +382,3 @@ TEST_F(SubprocessTest, TokenNotAvailable) {
 
   EXPECT_EQ(1u, subprocs_.finished_.size());
 }
-#endif  // _WIN32
diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-make-posix.cc
new file mode 100644
index 0000000000..70d84bfff7
--- /dev/null
+++ b/src/tokenpool-gnu-make-posix.cc
@@ -0,0 +1,203 @@
+// Copyright 2016-2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool-gnu-make.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <poll.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/time.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+// TokenPool implementation for GNU make jobserver - POSIX implementation
+// (http://make.mad-scientist.net/papers/jobserver-implementation/)
+struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
+  GNUmakeTokenPoolPosix();
+  virtual ~GNUmakeTokenPoolPosix();
+
+  virtual int GetMonitorFd();
+
+  virtual const char *GetEnv(const char *name) { return getenv(name); };
+  virtual bool ParseAuth(const char *jobserver);
+  virtual bool AcquireToken();
+  virtual bool ReturnToken();
+
+ private:
+  int rfd_;
+  int wfd_;
+
+  struct sigaction old_act_;
+  bool restore_;
+
+  static int dup_rfd_;
+  static void CloseDupRfd(int signum);
+
+  bool CheckFd(int fd);
+  bool SetAlarmHandler();
+};
+
+GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-1), restore_(false) {
+}
+
+GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
+  Clear();
+  if (restore_)
+    sigaction(SIGALRM, &old_act_, NULL);
+}
+
+bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
+  if (fd < 0)
+    return false;
+  int ret = fcntl(fd, F_GETFD);
+  if (ret < 0)
+    return false;
+  return true;
+}
+
+int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
+
+void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
+  close(dup_rfd_);
+  dup_rfd_ = -1;
+}
+
+bool GNUmakeTokenPoolPosix::SetAlarmHandler() {
+  struct sigaction act;
+  memset(&act, 0, sizeof(act));
+  act.sa_handler = CloseDupRfd;
+  if (sigaction(SIGALRM, &act, &old_act_) < 0) {
+    perror("sigaction:");
+    return(false);
+  } else {
+    restore_ = true;
+    return(true);
+  }
+}
+
+bool GNUmakeTokenPoolPosix::ParseAuth(const char *jobserver) {
+  int rfd = -1;
+  int wfd = -1;
+  if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
+      CheckFd(rfd) &&
+      CheckFd(wfd) &&
+      SetAlarmHandler()) {
+    rfd_ = rfd;
+    wfd_ = wfd;
+    return true;
+  }
+
+  return false;
+}
+
+bool GNUmakeTokenPoolPosix::AcquireToken() {
+  // Please read
+  //
+  //   http://make.mad-scientist.net/papers/jobserver-implementation/
+  //
+  // for the reasoning behind the following code.
+  //
+  // Try to read one character from the pipe. Returns true on success.
+  //
+  // First check if read() would succeed without blocking.
+#ifdef USE_PPOLL
+  pollfd pollfds[] = {{rfd_, POLLIN, 0}};
+  int ret = poll(pollfds, 1, 0);
+#else
+  fd_set set;
+  struct timeval timeout = { 0, 0 };
+  FD_ZERO(&set);
+  FD_SET(rfd_, &set);
+  int ret = select(rfd_ + 1, &set, NULL, NULL, &timeout);
+#endif
+  if (ret > 0) {
+    // Handle potential race condition:
+    //  - the above check succeeded, i.e. read() should not block
+    //  - the character disappears before we call read()
+    //
+    // Create a duplicate of rfd_. The duplicate file descriptor dup_rfd_
+    // can safely be closed by signal handlers without affecting rfd_.
+    dup_rfd_ = dup(rfd_);
+
+    if (dup_rfd_ != -1) {
+      struct sigaction act, old_act;
+      int ret = 0;
+
+      // Temporarily replace SIGCHLD handler with our own
+      memset(&act, 0, sizeof(act));
+      act.sa_handler = CloseDupRfd;
+      if (sigaction(SIGCHLD, &act, &old_act) == 0) {
+        struct itimerval timeout;
+
+        // install a 100ms timeout that generates SIGALARM on expiration
+        memset(&timeout, 0, sizeof(timeout));
+        timeout.it_value.tv_usec = 100 * 1000; // [ms] -> [usec]
+        if (setitimer(ITIMER_REAL, &timeout, NULL) == 0) {
+          char buf;
+
+          // Now try to read() from dup_rfd_. Return values from read():
+          //
+          // 1. token read                               ->  1
+          // 2. pipe closed                              ->  0
+          // 3. alarm expires                            -> -1 (EINTR)
+          // 4. child exits                              -> -1 (EINTR)
+          // 5. alarm expired before entering read()     -> -1 (EBADF)
+          // 6. child exited before entering read()      -> -1 (EBADF)
+          // 7. child exited before handler is installed -> go to 1 - 3
+          ret = read(dup_rfd_, &buf, 1);
+
+          // disarm timer
+          memset(&timeout, 0, sizeof(timeout));
+          setitimer(ITIMER_REAL, &timeout, NULL);
+        }
+
+        sigaction(SIGCHLD, &old_act, NULL);
+      }
+
+      CloseDupRfd(0);
+
+      // Case 1 from above list
+      if (ret > 0)
+        return true;
+    }
+  }
+
+  // read() would block, i.e. no token available,
+  // cases 2-6 from above list or
+  // select() / poll() / dup() / sigaction() / setitimer() failed
+  return false;
+}
+
+bool GNUmakeTokenPoolPosix::ReturnToken() {
+  const char buf = '+';
+  while (1) {
+    int ret = write(wfd_, &buf, 1);
+    if (ret > 0)
+      return true;
+    if ((ret != -1) || (errno != EINTR))
+      return false;
+    // write got interrupted - retry
+  }
+}
+
+int GNUmakeTokenPoolPosix::GetMonitorFd() {
+  return(rfd_);
+}
+
+struct TokenPool *TokenPool::Get() {
+  return new GNUmakeTokenPoolPosix;
+}
diff --git a/src/tokenpool-gnu-make-win32.cc b/src/tokenpool-gnu-make-win32.cc
new file mode 100644
index 0000000000..2719f2c1fc
--- /dev/null
+++ b/src/tokenpool-gnu-make-win32.cc
@@ -0,0 +1,237 @@
+// Copyright 2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool-gnu-make.h"
+
+// always include first to make sure other headers do the correct thing...
+#include <windows.h>
+
+#include <ctype.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "util.h"
+
+// TokenPool implementation for GNU make jobserver - Win32 implementation
+// (https://www.gnu.org/software/make/manual/html_node/Windows-Jobserver.html)
+struct GNUmakeTokenPoolWin32 : public GNUmakeTokenPool {
+  GNUmakeTokenPoolWin32();
+  virtual ~GNUmakeTokenPoolWin32();
+
+  virtual void WaitForTokenAvailability(HANDLE ioport);
+  virtual bool TokenIsAvailable(ULONG_PTR key);
+
+  virtual const char *GetEnv(const char *name);
+  virtual bool ParseAuth(const char *jobserver);
+  virtual bool AcquireToken();
+  virtual bool ReturnToken();
+
+ private:
+  // Semaphore for GNU make jobserver protocol
+  HANDLE semaphore_jobserver_;
+  // Semaphore Child -> Parent
+  // - child releases it before entering wait on jobserver semaphore
+  // - parent blocks on it to know when child enters wait
+  HANDLE semaphore_enter_wait_;
+  // Semaphore Parent -> Child
+  // - parent releases it to allow child to restart loop
+  // - child blocks on it to know when to restart loop
+  HANDLE semaphore_restart_;
+  // set to false if child should exit loop and terminate thread
+  bool running_;
+  // child thread
+  HANDLE child_;
+  // I/O completion port from SubprocessSet
+  HANDLE ioport_;
+
+
+  DWORD SemaphoreThread();
+  void ReleaseSemaphore(HANDLE semaphore);
+  void WaitForObject(HANDLE object);
+  static DWORD WINAPI SemaphoreThreadWrapper(LPVOID param);
+  static void NoopAPCFunc(ULONG_PTR param);
+};
+
+GNUmakeTokenPoolWin32::GNUmakeTokenPoolWin32() : semaphore_jobserver_(NULL),
+                                                 semaphore_enter_wait_(NULL),
+                                                 semaphore_restart_(NULL),
+                                                 running_(false),
+                                                 child_(NULL),
+                                                 ioport_(NULL) {
+}
+
+GNUmakeTokenPoolWin32::~GNUmakeTokenPoolWin32() {
+  Clear();
+  CloseHandle(semaphore_jobserver_);
+  semaphore_jobserver_ = NULL;
+
+  if (child_) {
+    // tell child thread to exit
+    running_ = false;
+    ReleaseSemaphore(semaphore_restart_);
+
+    // wait for child thread to exit
+    WaitForObject(child_);
+    CloseHandle(child_);
+    child_ = NULL;
+  }
+
+  if (semaphore_restart_) {
+    CloseHandle(semaphore_restart_);
+    semaphore_restart_ = NULL;
+  }
+
+  if (semaphore_enter_wait_) {
+    CloseHandle(semaphore_enter_wait_);
+    semaphore_enter_wait_ = NULL;
+  }
+}
+
+const char *GNUmakeTokenPoolWin32::GetEnv(const char *name) {
+  // getenv() does not work correctly together with tokenpool_tests.cc
+  static char buffer[MAX_PATH + 1];
+  if (GetEnvironmentVariable("MAKEFLAGS", buffer, sizeof(buffer)) == 0)
+    return NULL;
+  return(buffer);
+}
+
+bool GNUmakeTokenPoolWin32::ParseAuth(const char *jobserver) {
+  // match "--jobserver-auth=gmake_semaphore_<INTEGER>..."
+  const char *start = strchr(jobserver, '=');
+  if (start) {
+    const char *end = start;
+    unsigned int len;
+    char c, *auth;
+
+    while ((c = *++end) != '\0')
+      if (!(isalnum(c) || (c == '_')))
+        break;
+    len = end - start; // includes string terminator in count
+
+    if ((len > 1) && ((auth = (char *)malloc(len)) != NULL)) {
+      strncpy(auth, start + 1, len - 1);
+      auth[len - 1] = '\0';
+
+      if ((semaphore_jobserver_ = OpenSemaphore(SEMAPHORE_ALL_ACCESS, /* Semaphore access setting */
+                                                FALSE,                /* Child processes DON'T inherit */
+                                                auth                  /* Semaphore name */
+                                                )) != NULL) {
+        free(auth);
+        return true;
+      }
+
+      free(auth);
+    }
+  }
+
+  return false;
+}
+
+bool GNUmakeTokenPoolWin32::AcquireToken() {
+  return WaitForSingleObject(semaphore_jobserver_, 0) == WAIT_OBJECT_0;
+}
+
+bool GNUmakeTokenPoolWin32::ReturnToken() {
+  ReleaseSemaphore(semaphore_jobserver_);
+  return true;
+}
+
+DWORD GNUmakeTokenPoolWin32::SemaphoreThread() {
+  while (running_) {
+    // indicate to parent that we are entering wait
+    ReleaseSemaphore(semaphore_enter_wait_);
+
+    // alertable wait forever on token semaphore
+    if (WaitForSingleObjectEx(semaphore_jobserver_, INFINITE, TRUE) == WAIT_OBJECT_0) {
+      // release token again for AcquireToken()
+      ReleaseSemaphore(semaphore_jobserver_);
+
+      // indicate to parent on ioport that a token might be available
+      if (!PostQueuedCompletionStatus(ioport_, 0, (ULONG_PTR) this, NULL))
+        Win32Fatal("PostQueuedCompletionStatus");
+    }
+
+    // wait for parent to allow loop restart
+    WaitForObject(semaphore_restart_);
+    // semaphore is now in nonsignaled state again for next run...
+  }
+
+  return 0;
+}
+
+DWORD WINAPI GNUmakeTokenPoolWin32::SemaphoreThreadWrapper(LPVOID param) {
+  GNUmakeTokenPoolWin32 *This = (GNUmakeTokenPoolWin32 *) param;
+  return This->SemaphoreThread();
+}
+
+void GNUmakeTokenPoolWin32::NoopAPCFunc(ULONG_PTR param) {
+}
+
+void GNUmakeTokenPoolWin32::WaitForTokenAvailability(HANDLE ioport) {
+  if (child_ == NULL) {
+    // first invocation
+    //
+    // subprocess-win32.cc uses I/O completion port (IOCP) which can't be
+    // used as a waitable object. Therefore we can't use WaitMultipleObjects()
+    // to wait on the IOCP and the token semaphore at the same time. Create
+    // a child thread that waits on the semaphore and posts an I/O completion
+    ioport_ = ioport;
+
+    // create both semaphores in nonsignaled state
+    if ((semaphore_enter_wait_ = CreateSemaphore(NULL, 0, 1, NULL))
+        == NULL)
+      Win32Fatal("CreateSemaphore/enter_wait");
+    if ((semaphore_restart_ = CreateSemaphore(NULL, 0, 1, NULL))
+        == NULL)
+      Win32Fatal("CreateSemaphore/restart");
+
+    // start child thread
+    running_ = true;
+    if ((child_ = CreateThread(NULL, 0, &SemaphoreThreadWrapper, this, 0, NULL))
+        == NULL)
+      Win32Fatal("CreateThread");
+
+  } else {
+    // all further invocations - allow child thread to loop
+    ReleaseSemaphore(semaphore_restart_);
+  }
+
+  // wait for child thread to enter wait
+  WaitForObject(semaphore_enter_wait_);
+  // semaphore is now in nonsignaled state again for next run...
+
+  // now SubprocessSet::DoWork() can enter GetQueuedCompletionStatus()...
+}
+
+bool GNUmakeTokenPoolWin32::TokenIsAvailable(ULONG_PTR key) {
+  // alert child thread to break wait on token semaphore
+  QueueUserAPC(&NoopAPCFunc, child_, (ULONG_PTR)NULL);
+
+  // return true when GetQueuedCompletionStatus() returned our key
+  return key == (ULONG_PTR) this;
+}
+
+void GNUmakeTokenPoolWin32::ReleaseSemaphore(HANDLE semaphore) {
+  if (!::ReleaseSemaphore(semaphore, 1, NULL))
+    Win32Fatal("ReleaseSemaphore");
+}
+
+void GNUmakeTokenPoolWin32::WaitForObject(HANDLE object) {
+  if (WaitForSingleObject(object, INFINITE) != WAIT_OBJECT_0)
+    Win32Fatal("WaitForSingleObject");
+}
+
+struct TokenPool *TokenPool::Get() {
+  return new GNUmakeTokenPoolWin32;
+}
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index 4132bb06d9..92ff611721 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -12,101 +12,26 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "tokenpool.h"
+#include "tokenpool-gnu-make.h"
 
-#include <errno.h>
-#include <fcntl.h>
-#include <poll.h>
-#include <unistd.h>
-#include <signal.h>
-#include <sys/time.h>
+#include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
-#include <stdlib.h>
 
 #include "line_printer.h"
 
-// TokenPool implementation for GNU make jobserver
-// (http://make.mad-scientist.net/papers/jobserver-implementation/)
-struct GNUmakeTokenPool : public TokenPool {
-  GNUmakeTokenPool();
-  virtual ~GNUmakeTokenPool();
-
-  virtual bool Acquire();
-  virtual void Reserve();
-  virtual void Release();
-  virtual void Clear();
-  virtual int GetMonitorFd();
-
-  bool Setup(bool ignore, bool verbose, double& max_load_average);
-
- private:
-  int available_;
-  int used_;
-
-#ifdef _WIN32
-  // @TODO
-#else
-  int rfd_;
-  int wfd_;
-
-  struct sigaction old_act_;
-  bool restore_;
-
-  static int dup_rfd_;
-  static void CloseDupRfd(int signum);
-
-  bool CheckFd(int fd);
-  bool SetAlarmHandler();
-#endif
-
-  void Return();
-};
-
+// TokenPool implementation for GNU make jobserver - common bits
 // every instance owns an implicit token -> available_ == 1
-GNUmakeTokenPool::GNUmakeTokenPool() : available_(1), used_(0),
-                                       rfd_(-1), wfd_(-1), restore_(false) {
+GNUmakeTokenPool::GNUmakeTokenPool() : available_(1), used_(0) {
 }
 
 GNUmakeTokenPool::~GNUmakeTokenPool() {
-  Clear();
-  if (restore_)
-    sigaction(SIGALRM, &old_act_, NULL);
-}
-
-bool GNUmakeTokenPool::CheckFd(int fd) {
-  if (fd < 0)
-    return false;
-  int ret = fcntl(fd, F_GETFD);
-  if (ret < 0)
-    return false;
-  return true;
-}
-
-int GNUmakeTokenPool::dup_rfd_ = -1;
-
-void GNUmakeTokenPool::CloseDupRfd(int signum) {
-  close(dup_rfd_);
-  dup_rfd_ = -1;
-}
-
-bool GNUmakeTokenPool::SetAlarmHandler() {
-  struct sigaction act;
-  memset(&act, 0, sizeof(act));
-  act.sa_handler = CloseDupRfd;
-  if (sigaction(SIGALRM, &act, &old_act_) < 0) {
-    perror("sigaction:");
-    return(false);
-  } else {
-    restore_ = true;
-    return(true);
-  }
 }
 
 bool GNUmakeTokenPool::Setup(bool ignore,
                              bool verbose,
                              double& max_load_average) {
-  const char *value = getenv("MAKEFLAGS");
+  const char *value = GetEnv("MAKEFLAGS");
   if (value) {
     // GNU make <= 4.1
     const char *jobserver = strstr(value, "--jobserver-fds=");
@@ -119,20 +44,13 @@ bool GNUmakeTokenPool::Setup(bool ignore,
       if (ignore) {
         printer.PrintOnNewLine("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
       } else {
-        int rfd = -1;
-        int wfd = -1;
-        if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
-            CheckFd(rfd) &&
-            CheckFd(wfd) &&
-            SetAlarmHandler()) {
+        if (ParseAuth(jobserver)) {
           const char *l_arg = strstr(value, " -l");
           int load_limit = -1;
 
           if (verbose) {
             printer.PrintOnNewLine("ninja: using GNU make jobserver.\n");
           }
-          rfd_ = rfd;
-          wfd_ = wfd;
 
           // translate GNU make -lN to ninja -lN
           if (l_arg &&
@@ -154,83 +72,14 @@ bool GNUmakeTokenPool::Acquire() {
   if (available_ > 0)
     return true;
 
-  // Please read
-  //
-  //   http://make.mad-scientist.net/papers/jobserver-implementation/
-  //
-  // for the reasoning behind the following code.
-  //
-  // Try to read one character from the pipe. Returns true on success.
-  //
-  // First check if read() would succeed without blocking.
-#ifdef USE_PPOLL
-  pollfd pollfds[] = {{rfd_, POLLIN, 0}};
-  int ret = poll(pollfds, 1, 0);
-#else
-  fd_set set;
-  struct timeval timeout = { 0, 0 };
-  FD_ZERO(&set);
-  FD_SET(rfd_, &set);
-  int ret = select(rfd_ + 1, &set, NULL, NULL, &timeout);
-#endif
-  if (ret > 0) {
-    // Handle potential race condition:
-    //  - the above check succeeded, i.e. read() should not block
-    //  - the character disappears before we call read()
-    //
-    // Create a duplicate of rfd_. The duplicate file descriptor dup_rfd_
-    // can safely be closed by signal handlers without affecting rfd_.
-    dup_rfd_ = dup(rfd_);
-
-    if (dup_rfd_ != -1) {
-      struct sigaction act, old_act;
-      int ret = 0;
-
-      // Temporarily replace SIGCHLD handler with our own
-      memset(&act, 0, sizeof(act));
-      act.sa_handler = CloseDupRfd;
-      if (sigaction(SIGCHLD, &act, &old_act) == 0) {
-        struct itimerval timeout;
-
-        // install a 100ms timeout that generates SIGALARM on expiration
-        memset(&timeout, 0, sizeof(timeout));
-        timeout.it_value.tv_usec = 100 * 1000; // [ms] -> [usec]
-        if (setitimer(ITIMER_REAL, &timeout, NULL) == 0) {
-          char buf;
-
-          // Now try to read() from dup_rfd_. Return values from read():
-          //
-          // 1. token read                               ->  1
-          // 2. pipe closed                              ->  0
-          // 3. alarm expires                            -> -1 (EINTR)
-          // 4. child exits                              -> -1 (EINTR)
-          // 5. alarm expired before entering read()     -> -1 (EBADF)
-          // 6. child exited before entering read()      -> -1 (EBADF)
-          // 7. child exited before handler is installed -> go to 1 - 3
-          ret = read(dup_rfd_, &buf, 1);
-
-          // disarm timer
-          memset(&timeout, 0, sizeof(timeout));
-          setitimer(ITIMER_REAL, &timeout, NULL);
-        }
-
-        sigaction(SIGCHLD, &old_act, NULL);
-      }
-
-      CloseDupRfd(0);
-
-      // Case 1 from above list
-      if (ret > 0) {
-        available_++;
-        return true;
-      }
-    }
+  if (AcquireToken()) {
+    // token acquired
+    available_++;
+    return true;
+  } else {
+    // no token available
+    return false;
   }
-
-  // read() would block, i.e. no token available,
-  // cases 2-6 from above list or
-  // select() / poll() / dup() / sigaction() / setitimer() failed
-  return false;
 }
 
 void GNUmakeTokenPool::Reserve() {
@@ -239,15 +88,8 @@ void GNUmakeTokenPool::Reserve() {
 }
 
 void GNUmakeTokenPool::Return() {
-  const char buf = '+';
-  while (1) {
-    int ret = write(wfd_, &buf, 1);
-    if (ret > 0)
-      available_--;
-    if ((ret != -1) || (errno != EINTR))
-      return;
-    // write got interrupted - retry
-  }
+  if (ReturnToken())
+    available_--;
 }
 
 void GNUmakeTokenPool::Release() {
@@ -263,18 +105,3 @@ void GNUmakeTokenPool::Clear() {
   while (available_ > 1)
     Return();
 }
-
-int GNUmakeTokenPool::GetMonitorFd() {
-  return(rfd_);
-}
-
-struct TokenPool *TokenPool::Get(bool ignore,
-                                 bool verbose,
-                                 double& max_load_average) {
-  GNUmakeTokenPool *tokenpool = new GNUmakeTokenPool;
-  if (tokenpool->Setup(ignore, verbose, max_load_average))
-    return tokenpool;
-  else
-    delete tokenpool;
-  return NULL;
-}
diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
new file mode 100644
index 0000000000..d3852088e2
--- /dev/null
+++ b/src/tokenpool-gnu-make.h
@@ -0,0 +1,40 @@
+// Copyright 2016-2018 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tokenpool.h"
+
+// interface to GNU make token pool
+struct GNUmakeTokenPool : public TokenPool {
+  GNUmakeTokenPool();
+  virtual ~GNUmakeTokenPool();
+
+  // token pool implementation
+  virtual bool Acquire();
+  virtual void Reserve();
+  virtual void Release();
+  virtual void Clear();
+  virtual bool Setup(bool ignore, bool verbose, double& max_load_average);
+
+  // platform specific implementation
+  virtual const char *GetEnv(const char *name) = 0;
+  virtual bool ParseAuth(const char *jobserver) = 0;
+  virtual bool AcquireToken() = 0;
+  virtual bool ReturnToken() = 0;
+
+ private:
+  int available_;
+  int used_;
+
+  void Return();
+};
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
index 4c592875b4..613d16882d 100644
--- a/src/tokenpool-none.cc
+++ b/src/tokenpool-none.cc
@@ -17,8 +17,6 @@
 #include <stdlib.h>
 
 // No-op TokenPool implementation
-struct TokenPool *TokenPool::Get(bool ignore,
-                                 bool verbose,
-                                 double& max_load_average) {
+struct TokenPool *TokenPool::Get() {
   return NULL;
 }
diff --git a/src/tokenpool.h b/src/tokenpool.h
index 4bf477f20c..1be8e1d5ce 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -1,4 +1,4 @@
-// Copyright 2016-2017 Google Inc. All Rights Reserved.
+// Copyright 2016-2018 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,6 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#ifdef _WIN32
+#include <windows.h>
+#endif
+
 // interface to token pool
 struct TokenPool {
   virtual ~TokenPool() {}
@@ -21,14 +25,18 @@ struct TokenPool {
   virtual void Release() = 0;
   virtual void Clear() = 0;
 
+  // returns false if token pool setup failed
+  virtual bool Setup(bool ignore, bool verbose, double& max_load_average) = 0;
+
 #ifdef _WIN32
-  // @TODO
+  virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
+  // returns true if a token has become available
+  // key is result from GetQueuedCompletionStatus()
+  virtual bool TokenIsAvailable(ULONG_PTR key) = 0;
 #else
   virtual int GetMonitorFd() = 0;
 #endif
 
   // returns NULL if token pool is not available
-  static struct TokenPool *Get(bool ignore,
-                               bool verbose,
-                               double& max_load_average);
+  static struct TokenPool *Get();
 };
diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
index 6c89064ca4..8d4fd7d33a 100644
--- a/src/tokenpool_test.cc
+++ b/src/tokenpool_test.cc
@@ -16,13 +16,25 @@
 
 #include "test.h"
 
-#ifndef _WIN32
+#ifdef _WIN32
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 
+#ifdef _WIN32
+// should contain all valid characters
+#define SEMAPHORE_NAME      "abcdefghijklmnopqrstwxyz01234567890_"
+#define AUTH_FORMAT(tmpl)   "foo " tmpl "=%s bar"
+#define ENVIRONMENT_CLEAR() SetEnvironmentVariable("MAKEFLAGS", NULL)
+#define ENVIRONMENT_INIT(v) SetEnvironmentVariable("MAKEFLAGS", v)
+#else
+#define AUTH_FORMAT(tmpl)   "foo " tmpl "=%d,%d bar"
 #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
-#define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true);
+#define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
 #endif
 
 namespace {
@@ -32,43 +44,60 @@ const double kLoadAverageDefault = -1.23456789;
 struct TokenPoolTest : public testing::Test {
   double load_avg_;
   TokenPool *tokens_;
-#ifndef _WIN32
   char buf_[1024];
+#ifdef _WIN32
+  const char *semaphore_name_;
+  HANDLE semaphore_;
+#else
   int fds_[2];
 #endif
 
   virtual void SetUp() {
     load_avg_ = kLoadAverageDefault;
     tokens_ = NULL;
-#ifndef _WIN32
     ENVIRONMENT_CLEAR();
+#ifdef _WIN32
+    semaphore_name_ = SEMAPHORE_NAME;
+    if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME)) == NULL)
+#else
     if (pipe(fds_) < 0)
-      ASSERT_TRUE(false);
 #endif
+      ASSERT_TRUE(false);
   }
 
-  void CreatePool(const char *format, bool ignore_jobserver) {
-#ifndef _WIN32
+  void CreatePool(const char *format, bool ignore_jobserver = false) {
     if (format) {
-      sprintf(buf_, format, fds_[0], fds_[1]);
+      sprintf(buf_, format,
+#ifdef _WIN32
+              semaphore_name_
+#else
+              fds_[0], fds_[1]
+#endif
+      );
       ENVIRONMENT_INIT(buf_);
     }
-#endif
-    tokens_ = TokenPool::Get(ignore_jobserver, false, load_avg_);
+    if ((tokens_ = TokenPool::Get()) != NULL) {
+      if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
+        delete tokens_;
+        tokens_ = NULL;
+      }
+    }
   }
 
   void CreateDefaultPool() {
-    CreatePool("foo --jobserver-auth=%d,%d bar", false);
+    CreatePool(AUTH_FORMAT("--jobserver-auth"));
   }
 
   virtual void TearDown() {
     if (tokens_)
       delete tokens_;
-#ifndef _WIN32
+#ifdef _WIN32
+    CloseHandle(semaphore_);
+#else
     close(fds_[0]);
     close(fds_[1]);
-    ENVIRONMENT_CLEAR();
 #endif
+    ENVIRONMENT_CLEAR();
   }
 };
 
@@ -82,10 +111,9 @@ TEST_F(TokenPoolTest, NoTokenPool) {
   EXPECT_EQ(kLoadAverageDefault, load_avg_);
 }
 
-#ifndef _WIN32
 TEST_F(TokenPoolTest, SuccessfulOldSetup) {
   // GNUmake <= 4.1
-  CreatePool("foo --jobserver-fds=%d,%d bar", false);
+  CreatePool(AUTH_FORMAT("--jobserver-fds"));
 
   EXPECT_NE(NULL, tokens_);
   EXPECT_EQ(kLoadAverageDefault, load_avg_);
@@ -100,19 +128,37 @@ TEST_F(TokenPoolTest, SuccessfulNewSetup) {
 }
 
 TEST_F(TokenPoolTest, IgnoreWithJN) {
-  CreatePool("foo --jobserver-auth=%d,%d bar", true);
+  CreatePool(AUTH_FORMAT("--jobserver-auth"), true);
 
   EXPECT_EQ(NULL, tokens_);
   EXPECT_EQ(kLoadAverageDefault, load_avg_);
 }
 
 TEST_F(TokenPoolTest, HonorLN) {
-  CreatePool("foo -l9 --jobserver-auth=%d,%d bar", false);
+  CreatePool(AUTH_FORMAT("-l9 --jobserver-auth"));
 
   EXPECT_NE(NULL, tokens_);
   EXPECT_EQ(9.0, load_avg_);
 }
 
+#ifdef _WIN32
+TEST_F(TokenPoolTest, SemaphoreNotFound) {
+  semaphore_name_ = SEMAPHORE_NAME "_foobar";
+  CreateDefaultPool();
+
+  EXPECT_EQ(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+}
+
+TEST_F(TokenPoolTest, TokenIsAvailable) {
+  CreateDefaultPool();
+
+  ASSERT_NE(NULL, tokens_);
+  EXPECT_EQ(kLoadAverageDefault, load_avg_);
+
+  EXPECT_TRUE(tokens_->TokenIsAvailable((ULONG_PTR)tokens_));
+}
+#else
 TEST_F(TokenPoolTest, MonitorFD) {
   CreateDefaultPool();
 
@@ -121,6 +167,7 @@ TEST_F(TokenPoolTest, MonitorFD) {
 
   EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
 }
+#endif
 
 TEST_F(TokenPoolTest, ImplicitToken) {
   CreateDefaultPool();
@@ -147,7 +194,13 @@ TEST_F(TokenPoolTest, TwoTokens) {
   EXPECT_FALSE(tokens_->Acquire());
 
   // jobserver offers 2nd token
+#ifdef _WIN32
+  LONG previous;
+  ASSERT_TRUE(ReleaseSemaphore(semaphore_, 1, &previous));
+  ASSERT_EQ(0, previous);
+#else
   ASSERT_EQ(1u, write(fds_[1], "T", 1));
+#endif
   EXPECT_TRUE(tokens_->Acquire());
   tokens_->Reserve();
   EXPECT_FALSE(tokens_->Acquire());
@@ -160,8 +213,14 @@ TEST_F(TokenPoolTest, TwoTokens) {
   tokens_->Release();
   EXPECT_TRUE(tokens_->Acquire());
 
-  // there must be one token in the pipe
+  // there must be one token available
+#ifdef _WIN32
+  EXPECT_EQ(WAIT_OBJECT_0, WaitForSingleObject(semaphore_, 0));
+  EXPECT_TRUE(ReleaseSemaphore(semaphore_, 1, &previous));
+  EXPECT_EQ(0, previous);
+#else
   EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
+#endif
 
   // implicit token
   EXPECT_TRUE(tokens_->Acquire());
@@ -179,7 +238,13 @@ TEST_F(TokenPoolTest, Clear) {
   EXPECT_FALSE(tokens_->Acquire());
 
   // jobserver offers 2nd & 3rd token
+#ifdef _WIN32
+  LONG previous;
+  ASSERT_TRUE(ReleaseSemaphore(semaphore_, 2, &previous));
+  ASSERT_EQ(0, previous);
+#else
   ASSERT_EQ(2u, write(fds_[1], "TT", 2));
+#endif
   EXPECT_TRUE(tokens_->Acquire());
   tokens_->Reserve();
   EXPECT_TRUE(tokens_->Acquire());
@@ -189,10 +254,16 @@ TEST_F(TokenPoolTest, Clear) {
   tokens_->Clear();
   EXPECT_TRUE(tokens_->Acquire());
 
-  // there must be two tokens in the pipe
+  // there must be two tokens available
+#ifdef _WIN32
+  EXPECT_EQ(WAIT_OBJECT_0, WaitForSingleObject(semaphore_, 0));
+  EXPECT_EQ(WAIT_OBJECT_0, WaitForSingleObject(semaphore_, 0));
+  EXPECT_TRUE(ReleaseSemaphore(semaphore_, 2, &previous));
+  EXPECT_EQ(0, previous);
+#else
   EXPECT_EQ(2u, read(fds_[0], buf_, sizeof(buf_)));
+#endif
 
   // implicit token
   EXPECT_TRUE(tokens_->Acquire());
 }
-#endif

From 2b9c81c0ec1226d8795e7725529f13be41eaa385 Mon Sep 17 00:00:00 2001
From: Stefan Becker <chemobejk@gmail.com>
Date: Fri, 14 Dec 2018 13:27:11 +0200
Subject: [PATCH 11/11] Prepare PR for merging - part II

- remove unnecessary "struct" from TokenPool
- add PAPCFUNC cast to QueryUserAPC()
- remove hard-coded MAKEFLAGS string from win32
- remove useless build test CompleteNoWork
- rename TokenPoolTest to TestTokenPool
- add tokenpool modules to CMake build
- remove unused no-op TokenPool implementation
- fix errors flagged by codespell & clang-tidy
- address review comments from

https://github.com/ninja-build/ninja/pull/1140#pullrequestreview-195195803
https://github.com/ninja-build/ninja/pull/1140#pullrequestreview-185089255
https://github.com/ninja-build/ninja/pull/1140#issuecomment-473898963
https://github.com/ninja-build/ninja/pull/1140#issuecomment-596624610
---
 CMakeLists.txt                  |  8 ++++-
 src/build.cc                    |  2 +-
 src/build_test.cc               | 12 +------
 src/subprocess-posix.cc         |  4 +--
 src/subprocess-win32.cc         |  2 +-
 src/subprocess.h                |  2 +-
 src/subprocess_test.cc          | 26 +++++++-------
 src/tokenpool-gnu-make-posix.cc | 21 +++++------
 src/tokenpool-gnu-make-win32.cc | 36 ++++++++++---------
 src/tokenpool-gnu-make.cc       | 63 +++++++++++++++++----------------
 src/tokenpool-gnu-make.h        |  6 ++--
 src/tokenpool-none.cc           | 22 ------------
 src/tokenpool.h                 |  2 +-
 src/tokenpool_test.cc           |  8 ++---
 14 files changed, 94 insertions(+), 120 deletions(-)
 delete mode 100644 src/tokenpool-none.cc

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 57ae548f5b..e2876fe413 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -112,6 +112,7 @@ add_library(libninja OBJECT
 	src/state.cc
 	src/status.cc
 	src/string_piece_util.cc
+	src/tokenpool-gnu-make.cc
 	src/util.cc
 	src/version.cc
 )
@@ -123,9 +124,13 @@ if(WIN32)
 		src/msvc_helper_main-win32.cc
 		src/getopt.c
 		src/minidump-win32.cc
+		src/tokenpool-gnu-make-win32.cc
 	)
 else()
-	target_sources(libninja PRIVATE src/subprocess-posix.cc)
+	target_sources(libninja PRIVATE
+		src/subprocess-posix.cc
+		src/tokenpool-gnu-make-posix.cc
+	)
 	if(CMAKE_SYSTEM_NAME STREQUAL "OS400" OR CMAKE_SYSTEM_NAME STREQUAL "AIX")
 		target_sources(libninja PRIVATE src/getopt.c)
 	endif()
@@ -204,6 +209,7 @@ if(BUILD_TESTING)
     src/string_piece_util_test.cc
     src/subprocess_test.cc
     src/test.cc
+    src/tokenpool_test.cc
     src/util_test.cc
   )
   if(WIN32)
diff --git a/src/build.cc b/src/build.cc
index 20c3bdc2a0..854df08c2a 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -467,7 +467,7 @@ struct RealCommandRunner : public CommandRunner {
   // copy of config_.max_load_average; can be modified by TokenPool setup
   double max_load_average_;
   SubprocessSet subprocs_;
-  TokenPool *tokens_;
+  TokenPool* tokens_;
   map<const Subprocess*, Edge*> subproc_to_edge_;
 };
 
diff --git a/src/build_test.cc b/src/build_test.cc
index dd41dfbe1d..8901c9518f 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -4098,7 +4098,7 @@ struct BuildTokenTest : public BuildTest {
   void ExpectWaitForCommand(int count, ...);
 
 private:
-  void EnqueueBooleans(vector<bool>& booleans, int count, va_list ao);
+  void EnqueueBooleans(vector<bool>& booleans, int count, va_list ap);
 };
 
 void BuildTokenTest::SetUp() {
@@ -4144,16 +4144,6 @@ void BuildTokenTest::EnqueueBooleans(vector<bool>& booleans, int count, va_list
   }
 }
 
-TEST_F(BuildTokenTest, CompleteNoWork) {
-  // plan should not execute anything
-  string err;
-
-  EXPECT_TRUE(builder_.Build(&err));
-  EXPECT_EQ("", err);
-
-  EXPECT_EQ(0u, token_command_runner_.commands_ran_.size());
-}
-
 TEST_F(BuildTokenTest, DoNotAquireToken) {
   // plan should execute one command
   string err;
diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc
index 74451b0be2..31839276c4 100644
--- a/src/subprocess-posix.cc
+++ b/src/subprocess-posix.cc
@@ -250,7 +250,7 @@ Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
 }
 
 #ifdef USE_PPOLL
-bool SubprocessSet::DoWork(struct TokenPool* tokens) {
+bool SubprocessSet::DoWork(TokenPool* tokens) {
   vector<pollfd> fds;
   nfds_t nfds = 0;
 
@@ -315,7 +315,7 @@ bool SubprocessSet::DoWork(struct TokenPool* tokens) {
 }
 
 #else  // !defined(USE_PPOLL)
-bool SubprocessSet::DoWork(struct TokenPool* tokens) {
+bool SubprocessSet::DoWork(TokenPool* tokens) {
   fd_set set;
   int nfds = 0;
   FD_ZERO(&set);
diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc
index ce3e2c20a4..2926e9a221 100644
--- a/src/subprocess-win32.cc
+++ b/src/subprocess-win32.cc
@@ -252,7 +252,7 @@ Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
   return subprocess;
 }
 
-bool SubprocessSet::DoWork(struct TokenPool* tokens) {
+bool SubprocessSet::DoWork(TokenPool* tokens) {
   DWORD bytes_read;
   Subprocess* subproc;
   OVERLAPPED* overlapped;
diff --git a/src/subprocess.h b/src/subprocess.h
index 9ea67ea477..1ec78171e8 100644
--- a/src/subprocess.h
+++ b/src/subprocess.h
@@ -86,7 +86,7 @@ struct SubprocessSet {
   ~SubprocessSet();
 
   Subprocess* Add(const std::string& command, bool use_console = false);
-  bool DoWork(struct TokenPool* tokens);
+  bool DoWork(TokenPool* tokens);
   Subprocess* NextFinished();
   void Clear();
 
diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
index f625963462..7b146f31be 100644
--- a/src/subprocess_test.cc
+++ b/src/subprocess_test.cc
@@ -35,7 +35,7 @@ const char* kSimpleCommand = "cmd /c dir \\";
 const char* kSimpleCommand = "ls /";
 #endif
 
-struct TokenPoolTest : public TokenPool {
+struct TestTokenPool : public TokenPool {
   bool Acquire()     { return false; }
   void Reserve()     {}
   void Release()     {}
@@ -58,7 +58,7 @@ struct TokenPoolTest : public TokenPool {
 
 struct SubprocessTest : public testing::Test {
   SubprocessSet subprocs_;
-  TokenPoolTest tokens_;
+  TestTokenPool tokens_;
 };
 
 }  // anonymous namespace
@@ -73,7 +73,7 @@ TEST_F(SubprocessTest, BadCommandStderr) {
     // Pretend we discovered that stderr was ready for writing.
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
@@ -89,7 +89,7 @@ TEST_F(SubprocessTest, NoSuchCommand) {
     // Pretend we discovered that stderr was ready for writing.
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
@@ -109,7 +109,7 @@ TEST_F(SubprocessTest, InterruptChild) {
   while (!subproc->Done()) {
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -135,7 +135,7 @@ TEST_F(SubprocessTest, InterruptChildWithSigTerm) {
   while (!subproc->Done()) {
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -161,7 +161,7 @@ TEST_F(SubprocessTest, InterruptChildWithSigHup) {
   while (!subproc->Done()) {
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
   EXPECT_EQ(ExitInterrupted, subproc->Finish());
 }
@@ -190,7 +190,7 @@ TEST_F(SubprocessTest, Console) {
     while (!subproc->Done()) {
       subprocs_.DoWork(NULL);
     }
-    ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+    ASSERT_FALSE(subprocs_.IsTokenAvailable());
 
     EXPECT_EQ(ExitSuccess, subproc->Finish());
   }
@@ -206,7 +206,7 @@ TEST_F(SubprocessTest, SetWithSingle) {
   while (!subproc->Done()) {
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
   ASSERT_EQ(ExitSuccess, subproc->Finish());
   ASSERT_NE("", subproc->GetOutput());
 
@@ -243,7 +243,7 @@ TEST_F(SubprocessTest, SetWithMulti) {
     ASSERT_GT(subprocs_.running_.size(), 0u);
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
   ASSERT_EQ(0u, subprocs_.running_.size());
   ASSERT_EQ(3u, subprocs_.finished_.size());
 
@@ -278,7 +278,7 @@ TEST_F(SubprocessTest, SetWithLots) {
   subprocs_.ResetTokenAvailable();
   while (!subprocs_.running_.empty())
     subprocs_.DoWork(NULL);
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
   for (size_t i = 0; i < procs.size(); ++i) {
     ASSERT_EQ(ExitSuccess, procs[i]->Finish());
     ASSERT_NE("", procs[i]->GetOutput());
@@ -298,7 +298,7 @@ TEST_F(SubprocessTest, ReadStdin) {
   while (!subproc->Done()) {
     subprocs_.DoWork(NULL);
   }
-  ASSERT_EQ(false, subprocs_.IsTokenAvailable());
+  ASSERT_FALSE(subprocs_.IsTokenAvailable());
   ASSERT_EQ(ExitSuccess, subproc->Finish());
   ASSERT_EQ(1u, subprocs_.finished_.size());
 }
@@ -322,7 +322,7 @@ TEST_F(SubprocessTest, TokenAvailable) {
   subprocs_.DoWork(&tokens_);
 #ifdef _WIN32
   tokens_._token_available = false;
-  // we need to loop here as we have no conrol where the token
+  // we need to loop here as we have no control where the token
   // I/O completion post ends up in the queue
   while (!subproc->Done() && !subprocs_.IsTokenAvailable()) {
     subprocs_.DoWork(&tokens_);
diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-make-posix.cc
index 70d84bfff7..353bda226a 100644
--- a/src/tokenpool-gnu-make-posix.cc
+++ b/src/tokenpool-gnu-make-posix.cc
@@ -32,8 +32,8 @@ struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
 
   virtual int GetMonitorFd();
 
-  virtual const char *GetEnv(const char *name) { return getenv(name); };
-  virtual bool ParseAuth(const char *jobserver);
+  virtual const char* GetEnv(const char* name) { return getenv(name); };
+  virtual bool ParseAuth(const char* jobserver);
   virtual bool AcquireToken();
   virtual bool ReturnToken();
 
@@ -64,9 +64,7 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
   if (fd < 0)
     return false;
   int ret = fcntl(fd, F_GETFD);
-  if (ret < 0)
-    return false;
-  return true;
+  return ret >= 0;
 }
 
 int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
@@ -82,14 +80,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler() {
   act.sa_handler = CloseDupRfd;
   if (sigaction(SIGALRM, &act, &old_act_) < 0) {
     perror("sigaction:");
-    return(false);
-  } else {
-    restore_ = true;
-    return(true);
+    return false;
   }
+  restore_ = true;
+  return true;
 }
 
-bool GNUmakeTokenPoolPosix::ParseAuth(const char *jobserver) {
+bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
   int rfd = -1;
   int wfd = -1;
   if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
@@ -195,9 +192,9 @@ bool GNUmakeTokenPoolPosix::ReturnToken() {
 }
 
 int GNUmakeTokenPoolPosix::GetMonitorFd() {
-  return(rfd_);
+  return rfd_;
 }
 
-struct TokenPool *TokenPool::Get() {
+TokenPool* TokenPool::Get() {
   return new GNUmakeTokenPoolPosix;
 }
diff --git a/src/tokenpool-gnu-make-win32.cc b/src/tokenpool-gnu-make-win32.cc
index 2719f2c1fc..b2bb52fadb 100644
--- a/src/tokenpool-gnu-make-win32.cc
+++ b/src/tokenpool-gnu-make-win32.cc
@@ -14,7 +14,8 @@
 
 #include "tokenpool-gnu-make.h"
 
-// always include first to make sure other headers do the correct thing...
+// Always include this first.
+// Otherwise the other system headers don't work correctly under Win32
 #include <windows.h>
 
 #include <ctype.h>
@@ -32,8 +33,8 @@ struct GNUmakeTokenPoolWin32 : public GNUmakeTokenPool {
   virtual void WaitForTokenAvailability(HANDLE ioport);
   virtual bool TokenIsAvailable(ULONG_PTR key);
 
-  virtual const char *GetEnv(const char *name);
-  virtual bool ParseAuth(const char *jobserver);
+  virtual const char* GetEnv(const char* name);
+  virtual bool ParseAuth(const char* jobserver);
   virtual bool AcquireToken();
   virtual bool ReturnToken();
 
@@ -98,19 +99,19 @@ GNUmakeTokenPoolWin32::~GNUmakeTokenPoolWin32() {
   }
 }
 
-const char *GNUmakeTokenPoolWin32::GetEnv(const char *name) {
+const char* GNUmakeTokenPoolWin32::GetEnv(const char* name) {
   // getenv() does not work correctly together with tokenpool_tests.cc
   static char buffer[MAX_PATH + 1];
-  if (GetEnvironmentVariable("MAKEFLAGS", buffer, sizeof(buffer)) == 0)
+  if (GetEnvironmentVariable(name, buffer, sizeof(buffer)) == 0)
     return NULL;
-  return(buffer);
+  return buffer;
 }
 
-bool GNUmakeTokenPoolWin32::ParseAuth(const char *jobserver) {
+bool GNUmakeTokenPoolWin32::ParseAuth(const char* jobserver) {
   // match "--jobserver-auth=gmake_semaphore_<INTEGER>..."
-  const char *start = strchr(jobserver, '=');
+  const char* start = strchr(jobserver, '=');
   if (start) {
-    const char *end = start;
+    const char* end = start;
     unsigned int len;
     char c, *auth;
 
@@ -119,14 +120,15 @@ bool GNUmakeTokenPoolWin32::ParseAuth(const char *jobserver) {
         break;
     len = end - start; // includes string terminator in count
 
-    if ((len > 1) && ((auth = (char *)malloc(len)) != NULL)) {
+    if ((len > 1) && ((auth = (char*)malloc(len)) != NULL)) {
       strncpy(auth, start + 1, len - 1);
       auth[len - 1] = '\0';
 
-      if ((semaphore_jobserver_ = OpenSemaphore(SEMAPHORE_ALL_ACCESS, /* Semaphore access setting */
-                                                FALSE,                /* Child processes DON'T inherit */
-                                                auth                  /* Semaphore name */
-                                                )) != NULL) {
+      if ((semaphore_jobserver_ =
+           OpenSemaphore(SEMAPHORE_ALL_ACCESS, /* Semaphore access setting */
+                         FALSE,                /* Child processes DON'T inherit */
+                         auth                  /* Semaphore name */
+                        )) != NULL) {
         free(auth);
         return true;
       }
@@ -171,7 +173,7 @@ DWORD GNUmakeTokenPoolWin32::SemaphoreThread() {
 }
 
 DWORD WINAPI GNUmakeTokenPoolWin32::SemaphoreThreadWrapper(LPVOID param) {
-  GNUmakeTokenPoolWin32 *This = (GNUmakeTokenPoolWin32 *) param;
+  GNUmakeTokenPoolWin32* This = (GNUmakeTokenPoolWin32*) param;
   return This->SemaphoreThread();
 }
 
@@ -216,7 +218,7 @@ void GNUmakeTokenPoolWin32::WaitForTokenAvailability(HANDLE ioport) {
 
 bool GNUmakeTokenPoolWin32::TokenIsAvailable(ULONG_PTR key) {
   // alert child thread to break wait on token semaphore
-  QueueUserAPC(&NoopAPCFunc, child_, (ULONG_PTR)NULL);
+  QueueUserAPC((PAPCFUNC)&NoopAPCFunc, child_, (ULONG_PTR)NULL);
 
   // return true when GetQueuedCompletionStatus() returned our key
   return key == (ULONG_PTR) this;
@@ -232,6 +234,6 @@ void GNUmakeTokenPoolWin32::WaitForObject(HANDLE object) {
     Win32Fatal("WaitForSingleObject");
 }
 
-struct TokenPool *TokenPool::Get() {
+TokenPool* TokenPool::Get() {
   return new GNUmakeTokenPoolWin32;
 }
diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
index 92ff611721..60e0552924 100644
--- a/src/tokenpool-gnu-make.cc
+++ b/src/tokenpool-gnu-make.cc
@@ -31,36 +31,37 @@ GNUmakeTokenPool::~GNUmakeTokenPool() {
 bool GNUmakeTokenPool::Setup(bool ignore,
                              bool verbose,
                              double& max_load_average) {
-  const char *value = GetEnv("MAKEFLAGS");
-  if (value) {
-    // GNU make <= 4.1
-    const char *jobserver = strstr(value, "--jobserver-fds=");
+  const char* value = GetEnv("MAKEFLAGS");
+  if (!value)
+    return false;
+
+  // GNU make <= 4.1
+  const char* jobserver = strstr(value, "--jobserver-fds=");
+  if (!jobserver)
     // GNU make => 4.2
-    if (!jobserver)
-      jobserver = strstr(value, "--jobserver-auth=");
-    if (jobserver) {
-      LinePrinter printer;
-
-      if (ignore) {
-        printer.PrintOnNewLine("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
-      } else {
-        if (ParseAuth(jobserver)) {
-          const char *l_arg = strstr(value, " -l");
-          int load_limit = -1;
-
-          if (verbose) {
-            printer.PrintOnNewLine("ninja: using GNU make jobserver.\n");
-          }
-
-          // translate GNU make -lN to ninja -lN
-          if (l_arg &&
-              (sscanf(l_arg + 3, "%d ", &load_limit) == 1) &&
-              (load_limit > 0)) {
-            max_load_average = load_limit;
-          }
-
-          return true;
+    jobserver = strstr(value, "--jobserver-auth=");
+  if (jobserver) {
+    LinePrinter printer;
+
+    if (ignore) {
+      printer.PrintOnNewLine("ninja: warning: -jN forced on command line; ignoring GNU make jobserver.\n");
+    } else {
+      if (ParseAuth(jobserver)) {
+        const char* l_arg = strstr(value, " -l");
+        int load_limit = -1;
+
+        if (verbose) {
+          printer.PrintOnNewLine("ninja: using GNU make jobserver.\n");
+        }
+
+        // translate GNU make -lN to ninja -lN
+        if (l_arg &&
+            (sscanf(l_arg + 3, "%d ", &load_limit) == 1) &&
+            (load_limit > 0)) {
+          max_load_average = load_limit;
         }
+
+        return true;
       }
     }
   }
@@ -76,10 +77,10 @@ bool GNUmakeTokenPool::Acquire() {
     // token acquired
     available_++;
     return true;
-  } else {
-    // no token available
-    return false;
   }
+
+  // no token available
+  return false;
 }
 
 void GNUmakeTokenPool::Reserve() {
diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
index d3852088e2..c94cca5e2d 100644
--- a/src/tokenpool-gnu-make.h
+++ b/src/tokenpool-gnu-make.h
@@ -17,7 +17,7 @@
 // interface to GNU make token pool
 struct GNUmakeTokenPool : public TokenPool {
   GNUmakeTokenPool();
-  virtual ~GNUmakeTokenPool();
+  ~GNUmakeTokenPool();
 
   // token pool implementation
   virtual bool Acquire();
@@ -27,8 +27,8 @@ struct GNUmakeTokenPool : public TokenPool {
   virtual bool Setup(bool ignore, bool verbose, double& max_load_average);
 
   // platform specific implementation
-  virtual const char *GetEnv(const char *name) = 0;
-  virtual bool ParseAuth(const char *jobserver) = 0;
+  virtual const char* GetEnv(const char* name) = 0;
+  virtual bool ParseAuth(const char* jobserver) = 0;
   virtual bool AcquireToken() = 0;
   virtual bool ReturnToken() = 0;
 
diff --git a/src/tokenpool-none.cc b/src/tokenpool-none.cc
deleted file mode 100644
index 613d16882d..0000000000
--- a/src/tokenpool-none.cc
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright 2016-2018 Google Inc. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "tokenpool.h"
-
-#include <stdlib.h>
-
-// No-op TokenPool implementation
-struct TokenPool *TokenPool::Get() {
-  return NULL;
-}
diff --git a/src/tokenpool.h b/src/tokenpool.h
index 1be8e1d5ce..931c22754d 100644
--- a/src/tokenpool.h
+++ b/src/tokenpool.h
@@ -38,5 +38,5 @@ struct TokenPool {
 #endif
 
   // returns NULL if token pool is not available
-  static struct TokenPool *Get();
+  static TokenPool* Get();
 };
diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
index 8d4fd7d33a..8d3061cb30 100644
--- a/src/tokenpool_test.cc
+++ b/src/tokenpool_test.cc
@@ -43,10 +43,10 @@ const double kLoadAverageDefault = -1.23456789;
 
 struct TokenPoolTest : public testing::Test {
   double load_avg_;
-  TokenPool *tokens_;
+  TokenPool* tokens_;
   char buf_[1024];
 #ifdef _WIN32
-  const char *semaphore_name_;
+  const char* semaphore_name_;
   HANDLE semaphore_;
 #else
   int fds_[2];
@@ -65,7 +65,7 @@ struct TokenPoolTest : public testing::Test {
       ASSERT_TRUE(false);
   }
 
-  void CreatePool(const char *format, bool ignore_jobserver = false) {
+  void CreatePool(const char* format, bool ignore_jobserver = false) {
     if (format) {
       sprintf(buf_, format,
 #ifdef _WIN32
@@ -209,7 +209,7 @@ TEST_F(TokenPoolTest, TwoTokens) {
   tokens_->Release();
   EXPECT_TRUE(tokens_->Acquire());
 
-  // release implict token - must return 2nd token back to jobserver
+  // release implicit token - must return 2nd token back to jobserver
   tokens_->Release();
   EXPECT_TRUE(tokens_->Acquire());