Skip to content

Commit f296b3c

Browse files
sawenzelclaude
andcommitted
Flush queued hits before an o2-sim worker exits
This is fixing a bug where o2-sim hangs when the hit merger starts after the sim workers have finished. A worker called _exit with hits still queued in its simdata socket, so they were lost. The data channel now has an infinite linger and the worker closes it before exiting, which blocks until the merger has taken the hits. A parent-death signal makes sure a blocked worker does not outlive the o2-sim process tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 89eeccd commit f296b3c

1 file changed

Lines changed: 29 additions & 16 deletions

File tree

‎run/O2SimDeviceRunner.cxx‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
#include <cmath>
3232
#include <csignal>
3333
#include <unistd.h>
34+
#ifdef __linux__
35+
#include <sys/prctl.h>
36+
#endif
3437
#include "SimPublishChannelHelper.h"
3538

3639
#include "rapidjson/document.h"
@@ -48,9 +51,9 @@ void sigaction_handler(int signal, siginfo_t* signal_info, void*)
4851
auto pid = getpid();
4952
LOG(info) << pid << " caught signal " << signal << " from source " << signal_info->si_pid;
5053
auto groupid = getpgrp();
51-
if (pid == gMasterProcess && signal_info->si_pid != gDriverProcess) {
54+
if (pid == gMasterProcess && (signal_info->si_pid != gDriverProcess || signal == SIGTERM)) {
5255
// master worker forwards signal to whole worker process group
53-
// (do this only if not coming from gDriverProcess since this uses killpg and already affected all children)
56+
// (always for SIGTERM, since a parent-death signal also carries the driver pid)
5457
killpg(pid, signal);
5558
} else {
5659
if (signal_info->si_pid != gDriverProcess) {
@@ -168,6 +171,7 @@ KernelSetup initSim(std::string transport, std::string primaddress, std::string
168171
auto datachannel = new fair::mq::Channel{"simdata", "push", factory};
169172
datachannel->Connect(mergeraddress);
170173
datachannel->Validate();
174+
datachannel->GetSocket().SetLinger(-1); // keep queued hits until the merger has taken them
171175
// the channels are setup
172176

173177
// init the sim object
@@ -393,6 +397,13 @@ int main(int argc, char* argv[])
393397
// we use the current process as one of the workers as it has nothing else to do
394398
auto pid = (i == nworkers - 1) ? 0 : fork();
395399
if (pid == 0) {
400+
#ifdef __linux__
401+
// terminate if our parent dies, so a worker never outlives the o2-sim process tree
402+
prctl(PR_SET_PDEATHSIG, SIGTERM);
403+
if (getppid() != ((i == nworkers - 1) ? driverPID : gMasterProcess)) {
404+
_exit(1);
405+
}
406+
#endif
396407
// Each worker can publish its progress/state on a ZMQ channel.
397408
// We actually use a push/pull mechanism to collect all messages in the
398409
// master worker which can then publish using PUB/SUB.
@@ -418,7 +429,7 @@ int main(int argc, char* argv[])
418429
}
419430

420431
// everyone else is getting a push socket for notifications
421-
auto pushchannel = o2::simpubsub::createPUBChannel(o2::simpubsub::getPublishAddress("o2sim-workerinternal", driverPID), "push");
432+
auto pushchannel = std::make_unique<fair::mq::Channel>(o2::simpubsub::createPUBChannel(o2::simpubsub::getPublishAddress("o2sim-workerinternal", driverPID), "push"));
422433

423434
// we will try to pin each worker to a particular CPU
424435
// this can be made configurable via environment variables??
@@ -428,7 +439,7 @@ int main(int argc, char* argv[])
428439

429440
std::stringstream worker;
430441
worker << "WORKER" << i;
431-
o2::simpubsub::publishMessage(pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "SETUP COMPLETED"));
442+
o2::simpubsub::publishMessage(*pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "SETUP COMPLETED"));
432443

433444
auto& conf = o2::conf::SimConfig::Instance();
434445

@@ -438,18 +449,28 @@ int main(int argc, char* argv[])
438449

439450
if (conf.asService()) {
440451
LOG(info) << "IN SERVICE MODE WAITING";
441-
o2::simpubsub::publishMessage(pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "AWAITING INPUT"));
452+
o2::simpubsub::publishMessage(*pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "AWAITING INPUT"));
442453
more = waitForControlInput(kernelSetup.workerID);
443454
usleep(100); // --> why? (probably to give the server some chance to come to a "serving" state)
444455
} else {
445-
o2::simpubsub::publishMessage(pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "TERMINATING"));
456+
o2::simpubsub::publishMessage(*pushchannel, o2::simpubsub::simStatusString(worker.str(), "STATUS", "TERMINATING"));
446457

447458
LOG(info) << "FINISHING";
448459
more = false;
449460
}
450461
}
451-
sleep(10); // ---> give some time for message to be delivered to merger (destructing too early might affect the ZQM buffers)
452-
// The process will in any case be terminated by the main o2-sim driver.
462+
// Close the worker transport; this blocks until the merger has taken all queued hits
463+
pushchannel.reset();
464+
delete kernelSetup.primchannel;
465+
delete kernelSetup.primstatuschannel;
466+
delete kernelSetup.datachannel;
467+
468+
if (i == nworkers - 1) {
469+
// the master reaps its forked workers
470+
int status, cpid;
471+
while ((cpid = wait(&status)) != -1) {
472+
}
473+
}
453474

454475
// destruct setup (using _exit due to problems in ROOT shutdown (segmentation violations)
455476
// Clearly at some moment, a more robust solution would be appreciated
@@ -458,14 +479,6 @@ int main(int argc, char* argv[])
458479
gChildProcesses.push_back(pid);
459480
}
460481
}
461-
int status, cpid;
462-
while ((cpid = wait(&status))) {
463-
// LOG(info) << "normal wait " << cpid << " returned ";
464-
if (cpid == -1) {
465-
break;
466-
}
467-
}
468-
_exit(0);
469482
} else {
470483
// This the solution where we setup an ordinary fair::mq::Device
471484
// (each if which will setup its own simulation). Parallelism

0 commit comments

Comments
 (0)