Skip to content

Commit 46972ab

Browse files
codebyterecodebytere
authored
refactor: align shutdown signal handling with upstream (electron#26559)
1 parent b4196ca commit 46972ab

3 files changed

Lines changed: 54 additions & 32 deletions

File tree

‎shell/browser/electron_browser_main_parts.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,10 @@ void ElectronBrowserMainParts::PostMainMessageLoopStart() {
504504
bluez::DBusBluezManagerWrapperLinux::Initialize();
505505
#endif
506506
#if defined(OS_POSIX)
507-
HandleShutdownSignals();
507+
// Exit in response to SIGINT, SIGTERM, etc.
508+
InstallShutdownSignalHandlers(
509+
base::BindOnce(&Browser::Quit, base::Unretained(Browser::Get())),
510+
content::GetUIThreadTaskRunner({}));
508511
#endif
509512
}
510513

‎shell/browser/electron_browser_main_parts.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ class ElectronBrowserMainParts : public content::BrowserMainParts {
112112
#if defined(OS_POSIX)
113113
// Set signal handlers.
114114
void HandleSIGCHLD();
115-
void HandleShutdownSignals();
115+
void InstallShutdownSignalHandlers(
116+
base::OnceCallback<void()> shutdown_callback,
117+
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
116118
#endif
117119

118120
#if defined(OS_MAC)

‎shell/browser/electron_browser_main_parts_posix.cc‎

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,32 @@ void SIGTERMHandler(int signal) {
8080

8181
class ShutdownDetector : public base::PlatformThread::Delegate {
8282
public:
83-
explicit ShutdownDetector(int shutdown_fd);
83+
explicit ShutdownDetector(
84+
int shutdown_fd,
85+
base::OnceCallback<void()> shutdown_callback,
86+
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
8487

88+
// base::PlatformThread::Delegate:
8589
void ThreadMain() override;
8690

8791
private:
8892
const int shutdown_fd_;
93+
base::OnceCallback<void()> shutdown_callback_;
94+
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
8995

9096
DISALLOW_COPY_AND_ASSIGN(ShutdownDetector);
9197
};
9298

93-
ShutdownDetector::ShutdownDetector(int shutdown_fd)
94-
: shutdown_fd_(shutdown_fd) {
99+
ShutdownDetector::ShutdownDetector(
100+
int shutdown_fd,
101+
base::OnceCallback<void()> shutdown_callback,
102+
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
103+
: shutdown_fd_(shutdown_fd),
104+
shutdown_callback_(std::move(shutdown_callback)),
105+
task_runner_(task_runner) {
95106
CHECK_NE(shutdown_fd_, -1);
107+
CHECK(!shutdown_callback_.is_null());
108+
CHECK(task_runner_);
96109
}
97110

98111
// These functions are used to help us diagnose crash dumps that happen
@@ -121,7 +134,7 @@ void ShutdownDetector::ThreadMain() {
121134
int signal;
122135
size_t bytes_read = 0;
123136
do {
124-
ssize_t ret = HANDLE_EINTR(
137+
const ssize_t ret = HANDLE_EINTR(
125138
read(shutdown_fd_, reinterpret_cast<char*>(&signal) + bytes_read,
126139
sizeof(signal) - bytes_read));
127140
if (ret < 0) {
@@ -137,13 +150,12 @@ void ShutdownDetector::ThreadMain() {
137150
} while (bytes_read < sizeof(signal));
138151
VLOG(1) << "Handling shutdown for signal " << signal << ".";
139152

140-
if (!base::PostTask(
141-
FROM_HERE, {BrowserThread::UI},
142-
base::BindOnce(&Browser::Quit, base::Unretained(Browser::Get())))) {
143-
// Without a UI thread to post the exit task to, there aren't many
144-
// options. Raise the signal again. The default handler will pick it up
153+
if (!task_runner_->PostTask(FROM_HERE,
154+
base::BindOnce(std::move(shutdown_callback_)))) {
155+
// Without a valid task runner to post the exit task to, there aren't many
156+
// options. Raise the signal again. The default handler will pick it up
145157
// and cause an ungraceful exit.
146-
RAW_LOG(WARNING, "No UI thread, exiting ungracefully.");
158+
RAW_LOG(WARNING, "No valid task runner, exiting ungracefully.");
147159
kill(getpid(), signal);
148160

149161
// The signal may be handled on another thread. Give that a chance to
@@ -173,30 +185,33 @@ void ElectronBrowserMainParts::HandleSIGCHLD() {
173185
CHECK_EQ(sigaction(SIGCHLD, &action, nullptr), 0);
174186
}
175187

176-
void ElectronBrowserMainParts::HandleShutdownSignals() {
188+
void ElectronBrowserMainParts::InstallShutdownSignalHandlers(
189+
base::OnceCallback<void()> shutdown_callback,
190+
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
177191
int pipefd[2];
178192
int ret = pipe(pipefd);
179193
if (ret < 0) {
180194
PLOG(DFATAL) << "Failed to create pipe";
181-
} else {
182-
g_pipe_pid = getpid();
183-
g_shutdown_pipe_read_fd = pipefd[0];
184-
g_shutdown_pipe_write_fd = pipefd[1];
185-
#if !defined(ADDRESS_SANITIZER) && !defined(KEEP_SHADOW_STACKS)
186-
const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 2;
195+
return;
196+
}
197+
g_pipe_pid = getpid();
198+
g_shutdown_pipe_read_fd = pipefd[0];
199+
g_shutdown_pipe_write_fd = pipefd[1];
200+
#if !defined(ADDRESS_SANITIZER)
201+
const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 2;
187202
#else
188-
// ASan instrumentation and -finstrument-functions (used for keeping the
189-
// shadow stacks) bloat the stack frames, so we need to increase the stack
190-
// size to avoid hitting the guard page.
191-
const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 4;
203+
// ASan instrumentation bloats the stack frames, so we need to increase the
204+
// stack size to avoid hitting the guard page.
205+
const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 4;
192206
#endif
193-
// TODO(viettrungluu,willchan): crbug.com/29675 - This currently leaks, so
194-
// if you change this, you'll probably need to change the suppression.
195-
if (!base::PlatformThread::CreateNonJoinable(
196-
kShutdownDetectorThreadStackSize,
197-
new ShutdownDetector(g_shutdown_pipe_read_fd))) {
198-
LOG(DFATAL) << "Failed to create shutdown detector task.";
199-
}
207+
ShutdownDetector* detector = new ShutdownDetector(
208+
g_shutdown_pipe_read_fd, std::move(shutdown_callback), task_runner);
209+
210+
// PlatformThread does not delete its delegate.
211+
ANNOTATE_LEAKING_OBJECT_PTR(detector);
212+
if (!base::PlatformThread::CreateNonJoinable(kShutdownDetectorThreadStackSize,
213+
detector)) {
214+
LOG(DFATAL) << "Failed to create shutdown detector task.";
200215
}
201216
// Setup signal handlers for shutdown AFTER shutdown pipe is setup because
202217
// it may be called right away after handler is set.
@@ -205,16 +220,18 @@ void ElectronBrowserMainParts::HandleShutdownSignals() {
205220
// needs to be reset in child processes. See
206221
// base/process_util_posix.cc:LaunchProcess.
207222

208-
// We need to handle SIGTERM, because that is how many POSIX-based distros ask
209-
// processes to quit gracefully at shutdown time.
223+
// We need to handle SIGTERM, because that is how many POSIX-based distros
224+
// ask processes to quit gracefully at shutdown time.
210225
struct sigaction action;
211226
memset(&action, 0, sizeof(action));
212227
action.sa_handler = SIGTERMHandler;
213228
CHECK_EQ(sigaction(SIGTERM, &action, nullptr), 0);
229+
214230
// Also handle SIGINT - when the user terminates the browser via Ctrl+C. If
215231
// the browser process is being debugged, GDB will catch the SIGINT first.
216232
action.sa_handler = SIGINTHandler;
217233
CHECK_EQ(sigaction(SIGINT, &action, nullptr), 0);
234+
218235
// And SIGHUP, for when the terminal disappears. On shutdown, many Linux
219236
// distros send SIGHUP, SIGTERM, and then SIGKILL.
220237
action.sa_handler = SIGHUPHandler;

0 commit comments

Comments
 (0)