Skip to content

Commit

Permalink
fix: void register signal handler for SIG_CHILD
Browse files Browse the repository at this point in the history
linuxdeepin/developer-center#4412

QProcess::execute will use waitid to get exit code of that process it
starts. Register SIG_CHILD and wait all dead childs will break this
mechanism, as QProcess will find out that there is no child to wait,
and report this situation as a child process exit by crash.

So I change the method of launch application to use a double fork, to
start process and detach. Just as the way `QProcess` get this job done
in its `startDetached` method.

As a quick fix, I change the signature of `launch` method as no one use
its return value currently. If this is need someday, we should use a
pipe to let double forked child process report the return value and
errno of its `exec` call.

Signed-off-by: black-desk <me@black-desk.cn>
  • Loading branch information
black-desk committed May 30, 2023
1 parent 851d948 commit 4d6ec82
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 35 deletions.
73 changes: 40 additions & 33 deletions src/modules/startmanager/startmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,12 @@ StartManager::StartManager(QObject *parent)
, m_autostartFiles(getAutostartList())
, m_isDBusCalled(false)
{
waitForDeadChild();
loadSysMemLimitConfig();
getDesktopToAutostartMap();
listenAutostartFileEvents();
startAutostartProgram();
}

static void sig_child(int signo)
{
int stat;
int pid;
while((pid = waitpid(-1, &stat, WNOHANG)) > 0);
}

void StartManager::waitForDeadChild()
{
signal(SIGCHLD, sig_child);
}

bool StartManager::addAutostart(const QString &desktop)
{
setIsDBusCalled(true);
Expand Down Expand Up @@ -381,8 +368,12 @@ bool StartManager::doLaunchAppWithOptions(QString desktopFile, uint32_t timestam
return true;
}

bool StartManager::launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp, QStringList files)
void StartManager::launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp, QStringList files)
{
// NOTE(black_desk): this function do not return the result. If this feature
// needed someday, we should use pipe to let that double forked child
// process to report the return value of execvpe.

QProcess process; // NOTE(black_desk): this QProcess not used to start, we
// have to manually fork and exec to set
// GIO_LAUNCHED_DESKTOP_FILE_PID.
Expand Down Expand Up @@ -431,7 +422,7 @@ bool StartManager::launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp
if (ret != 0) {
qCritical() << "wordexp failed, error code:" << ret;
wordfree(&words);
return false;
return;
}

for (int i = 0; i < (int)words.we_wordc; i++) {
Expand Down Expand Up @@ -467,38 +458,54 @@ bool StartManager::launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp
envs.insert("GIO_LAUNCHED_DESKTOP_FILE", QString::fromStdString(info->getDesktopFile()->getFilePath()));

qint64 pid = fork();
if (pid == 0) {
envs.insert("GIO_LAUNCHED_DESKTOP_FILE_PID", QByteArray::number(getpid()).constData());
auto argList = process.arguments();
char const * args[argList.length() + 2];
std::transform(argList.constBegin(), argList.constEnd(), args + 1, [](const QString& str){
if (pid == -1){
qCritical() << "failed to fork, errno" << errno;
return;
} else if (pid == 0) {
// process to exit after vfork exec success.
qint64 doubleForkPID = fork();
if (doubleForkPID == -1) {
qCritical() << "failed to fork, errno" << errno;
exit(-1);
} else if (doubleForkPID == 0) {
// App process
envs.insert("GIO_LAUNCHED_DESKTOP_FILE_PID", QByteArray::number(getpid()).constData());
auto argList = process.arguments();
char const * args[argList.length() + 2];
std::transform(argList.constBegin(), argList.constEnd(), args + 1, [](const QString& str){
auto byte = new QByteArray;
*byte = str.toUtf8();
auto tmp_buf = byte->data();
return tmp_buf;
});
auto arg0 = process.program().toLocal8Bit();
args[0] = arg0.constData();
args[process.arguments().length() + 1] = 0;
auto envStringList = envs.toStringList();
char const * envs[envStringList.length() + 1];
std::transform(envStringList.constBegin(), envStringList.constEnd(), envs, [](const QString& str){
});
auto arg0 = process.program().toLocal8Bit();
args[0] = arg0.constData();
args[process.arguments().length() + 1] = 0;
auto envStringList = envs.toStringList();
char const * envs[envStringList.length() + 1];
std::transform(envStringList.constBegin(), envStringList.constEnd(), envs, [](const QString& str){
auto byte = new QByteArray;
*byte = str.toUtf8();
auto tmp_buf = byte->data();
return tmp_buf;
});
envs[envStringList.length()] = 0;
execvpe(arg0.constData(), (char**)args, (char**)envs);
exit(-1);
});
envs[envStringList.length()] = 0;
::execvpe(arg0.constData(), (char**)args, (char**)envs);
qCritical() <<"failed to execve app, errno" << errno;
_exit(-1);
}
qDebug() << "double fork pid:" << doubleForkPID;
_exit(0);
} else {
qDebug() << "pid:" << pid;
waitpid(pid, nullptr, 0);
if (useProxy) {
qDebug() << "Launch the process[" << pid << "] by app proxy.";
dbusHandler->addProxyProc(pid);
}
return true;
return;
}
return false;
return;
}

bool StartManager::doRunCommandWithOptions(QString exe, QStringList args, QVariantMap options)
Expand Down
3 changes: 1 addition & 2 deletions src/modules/startmanager/startmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,10 @@ public Q_SLOTS:
void onAutoStartupPathChange(const QString &dirPath);

private:
void waitForDeadChild();
bool setAutostart(const QString &fileName, const bool value);
bool doLaunchAppWithOptions(const QString &desktopFile);
bool doLaunchAppWithOptions(QString desktopFile, uint32_t timestamp, QStringList files, QVariantMap options);
bool launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp, QStringList files);
void launch(DesktopInfo *info, QString cmdLine, uint32_t timestamp, QStringList files);
bool doRunCommandWithOptions(QString exe, QStringList args, QVariantMap options);
void waitCmd(DesktopInfo *info, QProcess *process, QString cmdName);
bool shouldUseProxy(QString appId);
Expand Down

0 comments on commit 4d6ec82

Please sign in to comment.