Skip to content

Commit 13f7b30

Browse files
authored
[LLVM] Remove the requirement for named pipe in jobserver (#169154)
Remove the requirement that the jobserver "fifo" is actually a named pipe. Named pipes are essentially stateless, and therefore carry a high risk of a killed process leaving the server with no tokens left, and no clear way to reclaim them. Therefore, multiple jobserver implementations use FUSE instead: - [nixos-jobserver](NixOS/nixpkgs#314888) (WIP) uses simple file on FUSE - [steve](https://gitweb.gentoo.org/proj/steve.git) uses a character device via CUSE - [guildmaster](https://codeberg.org/amonakov/guildmaster) uses a character device via CUSE This is compatible with GNU make and Ninja, since they do not check the file type, and seems to be the only solution that can achieve state tracking while preserving compatibility. CC @amonakov --------- Signed-off-by: Michał Górny <mgorny@gentoo.org>
1 parent cb43ae4 commit 13f7b30

File tree

2 files changed

+36
-9
lines changed

2 files changed

+36
-9
lines changed

llvm/lib/Support/Unix/Jobserver.inc

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,6 @@
1919
#include <unistd.h>
2020

2121
namespace {
22-
/// Returns true if the given file descriptor is a FIFO (named pipe).
23-
bool isFifo(int FD) {
24-
struct stat StatBuf;
25-
if (::fstat(FD, &StatBuf) != 0)
26-
return false;
27-
return S_ISFIFO(StatBuf.st_mode);
28-
}
29-
3022
/// Returns true if the given file descriptors are valid.
3123
bool areFdsValid(int ReadFD, int WriteFD) {
3224
if (ReadFD == -1 || WriteFD == -1)
@@ -75,7 +67,7 @@ JobserverClientImpl::JobserverClientImpl(const JobserverConfig &Config) {
7567
case JobserverConfig::PosixFifo:
7668
// Open the FIFO for reading. It must be non-blocking and close-on-exec.
7769
ReadFD = ::open(Config.Path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC);
78-
if (ReadFD < 0 || !isFifo(ReadFD)) {
70+
if (ReadFD < 0) {
7971
if (ReadFD >= 0)
8072
::close(ReadFD);
8173
ReadFD = -1;

llvm/unittests/Support/JobserverTest.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "llvm/Support/Jobserver.h"
1515
#include "llvm/Config/llvm-config.h"
1616
#include "llvm/Support/Debug.h"
17+
#include "llvm/Support/FileUtilities.h"
1718
#include "llvm/Support/Parallel.h"
1819
#include "llvm/Support/Program.h"
1920
#include "llvm/Support/ThreadPool.h"
@@ -221,6 +222,40 @@ TEST_F(JobserverClientTest, UnixClientFifo) {
221222
EXPECT_TRUE(S1.isValid());
222223
}
223224

225+
TEST_F(JobserverClientTest, UnixClientNonFifo) {
226+
// This test verifies that non-FIFO jobservers can be used, such as steve
227+
// or guildmaster.
228+
SmallString<64> F;
229+
std::error_code EC =
230+
sys::fs::createTemporaryFile("jobserver-test", "nonfifo", F);
231+
ASSERT_FALSE(EC);
232+
FileRemover Cleanup(F);
233+
234+
// Intentionally inserted \t in environment string.
235+
std::string Makeflags = " \t -j4\t \t--jobserver-auth=fifo:";
236+
Makeflags += F.c_str();
237+
ScopedEnvironment Env("MAKEFLAGS", Makeflags.c_str());
238+
239+
JobserverClient *Client = JobserverClient::getInstance();
240+
ASSERT_NE(Client, nullptr);
241+
242+
// Get the implicit token.
243+
JobSlot S1 = Client->tryAcquire();
244+
EXPECT_TRUE(S1.isValid());
245+
EXPECT_TRUE(S1.isImplicit());
246+
247+
// File is empty, next acquire fails.
248+
JobSlot S2 = Client->tryAcquire();
249+
EXPECT_FALSE(S2.isValid());
250+
251+
// Release does not write to the file for the implicit token.
252+
Client->release(std::move(S1));
253+
254+
// Re-acquire the implicit token.
255+
S1 = Client->tryAcquire();
256+
EXPECT_TRUE(S1.isValid());
257+
}
258+
224259
#if LLVM_ENABLE_THREADS
225260
// Unique anchor whose address helps locate the current test binary.
226261
static int JobserverTestAnchor = 0;

0 commit comments

Comments
 (0)