mirror of
https://github.com/NixOS/nix.git
synced 2025-11-12 13:36:02 +01:00
Move pseudoterminal slave open to child
Hopefully this fixes "unexpected EOF" failures on macOS
(#3137, #3605, #7242, #7702).
The problem appears to be that under some circumstances, macOS
discards the output written to the slave side of the
pseudoterminal. Hence the parent never sees the "sandbox initialized"
message from the child, even though it succeeded. The conditions are:
* The child finishes very quickly. That's why this bug is likely to
trigger in nix-env tests, since that uses a builtin builder. Adding
a short sleep before the child exits makes the problem go away.
* The parent has closed its duplicate of the slave file
descriptor. This shouldn't matter, since the child has a duplicate
as well, but it does. E.g. moving the close to the bottom of
startBuilder() makes the problem go away. However, that's not a
solution because it would make Nix hang if the child dies before
sending the "sandbox initialized" message.
* The system is under high load. E.g. "make installcheck -j16" makes
the issue pretty reproducible, while it's very rare under "make
installcheck -j1".
As a fix/workaround, we now open the pseudoterminal slave in the
child, rather than the parent. This removes the second condition
(i.e. the parent no longer needs to close the slave fd) and I haven't
been able to reproduce the "unexpected EOF" with this.
(cherry picked from commit c536e00c9d)
This commit is contained in:
parent
3a82f6a117
commit
6613e7ebfb
1 changed files with 29 additions and 27 deletions
|
|
@ -827,6 +827,10 @@ private:
|
||||||
/* Pipe for the builder's standard output/error. */
|
/* Pipe for the builder's standard output/error. */
|
||||||
Pipe builderOut;
|
Pipe builderOut;
|
||||||
|
|
||||||
|
/* Slave side of the pseudoterminal used for the builder's
|
||||||
|
standard output/error. */
|
||||||
|
Path slaveName;
|
||||||
|
|
||||||
/* Pipe for synchronising updates to the builder user namespace. */
|
/* Pipe for synchronising updates to the builder user namespace. */
|
||||||
Pipe userNamespaceSync;
|
Pipe userNamespaceSync;
|
||||||
|
|
||||||
|
|
@ -2238,14 +2242,12 @@ void DerivationGoal::startBuilder()
|
||||||
/* Create the log file. */
|
/* Create the log file. */
|
||||||
Path logFile = openLogFile();
|
Path logFile = openLogFile();
|
||||||
|
|
||||||
/* Create a pipe to get the output of the builder. */
|
/* Create a pseudoterminal to get the output of the builder. */
|
||||||
//builderOut.create();
|
|
||||||
|
|
||||||
builderOut.readSide = posix_openpt(O_RDWR | O_NOCTTY);
|
builderOut.readSide = posix_openpt(O_RDWR | O_NOCTTY);
|
||||||
if (!builderOut.readSide)
|
if (!builderOut.readSide)
|
||||||
throw SysError("opening pseudoterminal master");
|
throw SysError("opening pseudoterminal master");
|
||||||
|
|
||||||
std::string slaveName(ptsname(builderOut.readSide.get()));
|
slaveName = ptsname(builderOut.readSide.get());
|
||||||
|
|
||||||
if (buildUser) {
|
if (buildUser) {
|
||||||
if (chmod(slaveName.c_str(), 0600))
|
if (chmod(slaveName.c_str(), 0600))
|
||||||
|
|
@ -2258,30 +2260,9 @@ void DerivationGoal::startBuilder()
|
||||||
throw SysError("granting access to pseudoterminal slave");
|
throw SysError("granting access to pseudoterminal slave");
|
||||||
}
|
}
|
||||||
|
|
||||||
#if 0
|
|
||||||
// Mount the pt in the sandbox so that the "tty" command works.
|
|
||||||
// FIXME: this doesn't work with the new devpts in the sandbox.
|
|
||||||
if (useChroot)
|
|
||||||
dirsInChroot[slaveName] = {slaveName, false};
|
|
||||||
#endif
|
|
||||||
|
|
||||||
if (unlockpt(builderOut.readSide.get()))
|
if (unlockpt(builderOut.readSide.get()))
|
||||||
throw SysError("unlocking pseudoterminal");
|
throw SysError("unlocking pseudoterminal");
|
||||||
|
|
||||||
builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY);
|
|
||||||
if (!builderOut.writeSide)
|
|
||||||
throw SysError("opening pseudoterminal slave");
|
|
||||||
|
|
||||||
// Put the pt into raw mode to prevent \n -> \r\n translation.
|
|
||||||
struct termios term;
|
|
||||||
if (tcgetattr(builderOut.writeSide.get(), &term))
|
|
||||||
throw SysError("getting pseudoterminal attributes");
|
|
||||||
|
|
||||||
cfmakeraw(&term);
|
|
||||||
|
|
||||||
if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term))
|
|
||||||
throw SysError("putting pseudoterminal into raw mode");
|
|
||||||
|
|
||||||
result.startTime = time(0);
|
result.startTime = time(0);
|
||||||
|
|
||||||
/* Fork a child to build the package. */
|
/* Fork a child to build the package. */
|
||||||
|
|
@ -2330,7 +2311,11 @@ void DerivationGoal::startBuilder()
|
||||||
|
|
||||||
options.allowVfork = false;
|
options.allowVfork = false;
|
||||||
|
|
||||||
|
Pipe sendPid;
|
||||||
|
sendPid.create();
|
||||||
|
|
||||||
Pid helper = startProcess([&]() {
|
Pid helper = startProcess([&]() {
|
||||||
|
sendPid.readSide.close();
|
||||||
|
|
||||||
/* Drop additional groups here because we can't do it
|
/* Drop additional groups here because we can't do it
|
||||||
after we've created the new user namespace. FIXME:
|
after we've created the new user namespace. FIXME:
|
||||||
|
|
@ -2374,10 +2359,12 @@ void DerivationGoal::startBuilder()
|
||||||
_exit(1);
|
_exit(1);
|
||||||
if (child == -1) throw SysError("cloning builder process");
|
if (child == -1) throw SysError("cloning builder process");
|
||||||
|
|
||||||
writeFull(builderOut.writeSide.get(), std::to_string(child) + "\n");
|
writeFull(sendPid.writeSide.get(), std::to_string(child) + "\n");
|
||||||
_exit(0);
|
_exit(0);
|
||||||
}, options);
|
}, options);
|
||||||
|
|
||||||
|
sendPid.writeSide.close();
|
||||||
|
|
||||||
int res = helper.wait();
|
int res = helper.wait();
|
||||||
if (res != 0 && settings.sandboxFallback) {
|
if (res != 0 && settings.sandboxFallback) {
|
||||||
useChroot = false;
|
useChroot = false;
|
||||||
|
|
@ -2395,7 +2382,7 @@ void DerivationGoal::startBuilder()
|
||||||
});
|
});
|
||||||
|
|
||||||
pid_t tmp;
|
pid_t tmp;
|
||||||
if (!string2Int<pid_t>(readLine(builderOut.readSide.get()), tmp)) abort();
|
if (!string2Int<pid_t>(readLine(sendPid.readSide.get()), tmp)) abort();
|
||||||
pid = tmp;
|
pid = tmp;
|
||||||
|
|
||||||
/* Set the UID/GID mapping of the builder's user namespace
|
/* Set the UID/GID mapping of the builder's user namespace
|
||||||
|
|
@ -2736,6 +2723,21 @@ void DerivationGoal::runChild()
|
||||||
|
|
||||||
try { /* child */
|
try { /* child */
|
||||||
|
|
||||||
|
/* Open the slave side of the pseudoterminal. */
|
||||||
|
builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY);
|
||||||
|
if (!builderOut.writeSide)
|
||||||
|
throw SysError("opening pseudoterminal slave");
|
||||||
|
|
||||||
|
// Put the pt into raw mode to prevent \n -> \r\n translation.
|
||||||
|
struct termios term;
|
||||||
|
if (tcgetattr(builderOut.writeSide.get(), &term))
|
||||||
|
throw SysError("getting pseudoterminal attributes");
|
||||||
|
|
||||||
|
cfmakeraw(&term);
|
||||||
|
|
||||||
|
if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term))
|
||||||
|
throw SysError("putting pseudoterminal into raw mode");
|
||||||
|
|
||||||
commonChildInit(builderOut.writeSide.get());
|
commonChildInit(builderOut.writeSide.get());
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue