Only reset tty.tg to nil when its controlling process is being released.

This means that when tg is being released, IFF tg.tty.tg == tg (which means tg
was tg.tty's controlling process), then we can reset tty.tg to nil.

Otherwise, as shown in reproducers of #9898, when a non-controlling process
exits, it resets the TTY's tg field (which indicates the controlling thread
group) and subsequently the alive controlling thread group can no longer
receive signals from the TTY.

Fixes #9898

PiperOrigin-RevId: 600987817
This commit is contained in:
Ayush Ranjan
2024-01-23 20:22:06 -08:00
committed by gVisor bot
parent ff8ea9b2b9
commit f82d97c9ee
2 changed files with 58 additions and 2 deletions
+3 -1
View File
@@ -325,7 +325,9 @@ func (tg *ThreadGroup) Release(ctx context.Context) {
// Disassociate from the tty if we have one.
if tg.tty != nil {
tg.tty.mu.Lock()
tg.tty.tg = nil
if tg.tty.tg == tg {
tg.tty.tg = nil
}
tg.tty.mu.Unlock()
tg.tty = nil
}
+55 -1
View File
@@ -27,15 +27,16 @@
#include <termios.h>
#include <unistd.h>
#include <csignal>
#include <iostream>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/base/macros.h"
#include "absl/strings/str_cat.h"
#include "absl/synchronization/notification.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "test/util/capability_util.h"
#include "test/util/cleanup.h"
#include "test/util/file_descriptor.h"
#include "test/util/fs_util.h"
@@ -1673,6 +1674,59 @@ TEST_F(JobControlTest, ReleaseTTYSignals) {
EXPECT_THAT(kill(diff_pgrp_child, SIGKILL), SyscallSucceeds());
}
// Used by the child process spawned in
// ControllingProcessPersistsAfterChildExists to track received signals.
static int received2;
void sig_handler2(int signum) { received2 |= signum; }
// NOTE(gvisor.dev/issue/9898): Regression test. Tests that a TTY's controlling
// process is not cleared when a non-controlling process exits.
TEST_F(JobControlTest, ControllingProcessPersistsAfterChildExists) {
// Set the controlling process for the PTY.
ASSERT_THAT(ioctl(replica_.get(), TIOCSCTTY, 0), SyscallSucceeds());
// Fork a child, which does nothing and exits. We expect that this process
// is still the controlling process, so is capable of receiving signals.
ASSERT_NO_ERRNO(RunInChild([=]() {}));
// Install handler for SIGINT.
received2 = 0;
struct sigaction sa = {};
sa.sa_handler = sig_handler2;
sa.sa_flags = 0;
sigemptyset(&sa.sa_mask);
sigaddset(&sa.sa_mask, SIGINT);
ASSERT_THAT(sigaction(SIGINT, &sa, NULL), SyscallSucceeds());
// Send ^C.
constexpr char kInput = ControlCharacter('C');
ASSERT_THAT(WriteFd(master_.get(), &kInput, 1), SyscallSucceedsWithValue(1));
// Ensure we got the signal. Wait at most for kTimeout.
absl::Time end = absl::Now() + kTimeout;
while (received2 != SIGINT) {
absl::SleepFor(absl::Seconds(1));
if (end < absl::Now()) {
FAIL() << "Timed out waiting for SIGINT signal.";
break;
}
}
// Make sure we're ignoring SIGHUP, which will be sent to this process once we
// disconnect the TTY.
struct sigaction sighup_sa = {};
sighup_sa.sa_handler = SIG_IGN;
sighup_sa.sa_flags = 0;
sigemptyset(&sighup_sa.sa_mask);
struct sigaction old_sa;
EXPECT_THAT(sigaction(SIGHUP, &sighup_sa, &old_sa), SyscallSucceeds());
// Release the controlling terminal and restore the old SIGHUP handler.
EXPECT_THAT(ioctl(replica_.get(), TIOCNOTTY), SyscallSucceeds());
EXPECT_THAT(sigaction(SIGHUP, &old_sa, NULL), SyscallSucceeds());
}
TEST_F(JobControlTest, GetForegroundProcessGroup) {
auto res = RunInChild([=]() {
pid_t pid, foreground_pgid;