Skip to content

Commit

Permalink
fix: ensure utilityProcess only emits one 'exit' event (#44266)
Browse files Browse the repository at this point in the history
fix: ensure utilityProcess only emits one exit

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
  • Loading branch information
trop[bot] and codebytere authored Oct 16, 2024
1 parent 11c2b61 commit e1419b2
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
15 changes: 7 additions & 8 deletions shell/browser/api/electron_api_utility_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,16 @@ void UtilityProcessWrapper::OnServiceProcessLaunch(
}

void UtilityProcessWrapper::HandleTermination(uint64_t exit_code) {
// HandleTermination is called from multiple callsites,
// we need to ensure we only process it for the first callsite.
if (terminated_)
return;
terminated_ = true;

if (pid_ != base::kNullProcessId)
GetAllUtilityProcessWrappers().Remove(pid_);
CloseConnectorPort();

EmitWithoutEvent("exit", exit_code);

Unpin();
}

Expand Down Expand Up @@ -292,13 +296,8 @@ void UtilityProcessWrapper::CloseConnectorPort() {
}

void UtilityProcessWrapper::Shutdown(uint64_t exit_code) {
if (pid_ != base::kNullProcessId)
GetAllUtilityProcessWrappers().Remove(pid_);
node_service_remote_.reset();
CloseConnectorPort();
// Emit 'exit' event
EmitWithoutEvent("exit", exit_code);
Unpin();
HandleTermination(exit_code);
}

void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
Expand Down
1 change: 1 addition & 0 deletions shell/browser/api/electron_api_utility_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class UtilityProcessWrapper final
int stdout_read_fd_ = -1;
int stderr_read_fd_ = -1;
bool connector_closed_ = false;
bool terminated_ = false;
std::unique_ptr<mojo::Connector> connector_;
blink::MessagePortDescriptor host_port_;
mojo::Receiver<node::mojom::NodeServiceClient> receiver_{this};
Expand Down
16 changes: 13 additions & 3 deletions spec/api-utility-process-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,20 @@ describe('utilityProcess module', () => {
await once(child, 'spawn');
});

it('emits \'exit\' when child process exits gracefully', async () => {
it('emits \'exit\' when child process exits gracefully', (done) => {
const child = utilityProcess.fork(path.join(fixturesPath, 'empty.js'));
const [code] = await once(child, 'exit');
expect(code).to.equal(0);
child.on('exit', (code) => {
expect(code).to.equal(0);
done();
});
});

it('emits \'exit\' when the child process file does not exist', (done) => {
const child = utilityProcess.fork('nonexistent');
child.on('exit', (code) => {
expect(code).to.equal(1);
done();
});
});

ifit(!isWindows32Bit)('emits the correct error code when child process exits nonzero', async () => {
Expand Down

0 comments on commit e1419b2

Please sign in to comment.