Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

daemon: refactor IPC usage #664

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+28 −11
Open

daemon: refactor IPC usage #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9720b9d
daemon: refactor IPC usage
Jul 16, 2024
aa89eb4
update: add SIGSEGV into dlt_daemon_signal_handler
Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/cmake-ctest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ on:
env:
BUILD_TYPE: Release
WITH_DLT_COVERAGE: ON
WITH_DLT_SHM_ENABLE: ON
DLT_IPC: "UNIX_SOCKET"
BUILD_GMOCK: OFF

jobs:
Expand All @@ -27,6 +29,8 @@ jobs:
run: |
cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
-DWITH_DLT_COVERAGE=${{env.WITH_DLT_COVERAGE}} \
-DWITH_DLT_SHM_ENABLE=${{env.WITH_DLT_SHM_ENABLE}} \
-DDLT_IPC=${{env.DLT_IPC}} \
-DBUILD_GMOCK=${{env.BUILD_GMOCK}}

- name: Build
Expand Down
35 changes: 24 additions & 11 deletions src/daemon/dlt-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ int dlt_daemon_local_init_p1(DltDaemon *daemon, DltDaemonLocal *daemon_local, in
signal(SIGHUP, dlt_daemon_signal_handler); /* hangup signal */
signal(SIGQUIT, dlt_daemon_signal_handler);
signal(SIGINT, dlt_daemon_signal_handler);
signal(SIGSEGV, dlt_daemon_signal_handler);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handler for SIGSEGV should not be added here. This will cause the application to hang in an endless loop in case a segmentation fault occurs as it will re-execute the same instruction again which caused the segmentation fault (and signal is triggered again)

#ifdef __QNX__
signal(SIGUSR1, dlt_daemon_signal_handler); /* for timer threads */
#endif
Expand Down Expand Up @@ -1585,14 +1586,19 @@ static int dlt_daemon_init_fifo(DltDaemonLocal *daemon_local)
{
int ret;
int fd = -1;
int dir_fd = -1;
int fifo_size;

/* open named pipe(FIFO) to receive DLT messages from users */
umask(0);

/* Try to delete existing pipe, ignore result of unlink */
/* Valid fifo means there is a daemon running, stop init phase of the new */
const char *tmpFifo = daemon_local->flags.daemonFifoName;
unlink(tmpFifo);
if (access(tmpFifo, F_OK) == 0) {
dlt_vlog(LOG_WARNING, "FIFO user %s is in use (%s)!\n",
tmpFifo, strerror(errno));
return -1;
}

ret = mkfifo(tmpFifo, S_IRUSR | S_IWUSR | S_IWGRP);

Expand All @@ -1602,13 +1608,27 @@ static int dlt_daemon_init_fifo(DltDaemonLocal *daemon_local)
return -1;
} /* if */

dir_fd = open(dltFifoBaseDir, O_RDONLY);
if (dir_fd == -1) {
dlt_vlog(LOG_WARNING, "Directory %s of fifo cannot be opened (%s)!\n",
dltFifoBaseDir, strerror(errno));
return -1;
}

fd = openat(dir_fd, tmpFifo, O_RDWR);
if (fd == -1) {
dlt_vlog(LOG_WARNING, "FIFO user %s cannot be opened (%s)!\n",
tmpFifo, strerror(errno));
return -1;
} /* if */

/* Set group of daemon FIFO */
if (daemon_local->flags.daemonFifoGroup[0] != 0) {
errno = 0;
struct group *group_dlt = getgrnam(daemon_local->flags.daemonFifoGroup);

if (group_dlt) {
ret = chown(tmpFifo, -1, group_dlt->gr_gid);
ret = fchown(fd, -1, group_dlt->gr_gid);

if (ret == -1)
dlt_vlog(LOG_ERR, "FIFO user %s cannot be chowned to group %s (%s)\n",
Expand All @@ -1628,14 +1648,6 @@ static int dlt_daemon_init_fifo(DltDaemonLocal *daemon_local)
}
}

fd = open(tmpFifo, O_RDWR);

if (fd == -1) {
dlt_vlog(LOG_WARNING, "FIFO user %s cannot be opened (%s)!\n",
tmpFifo, strerror(errno));
return -1;
} /* if */

#ifdef __linux__
/* F_SETPIPE_SZ and F_GETPIPE_SZ are only supported for Linux.
* For other OSes it depends on its system e.g. pipe manager.
Expand Down Expand Up @@ -2150,6 +2162,7 @@ void dlt_daemon_signal_handler(int sig)
case SIGTERM:
case SIGINT:
case SIGQUIT:
case SIGSEGV:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIGSEGV needs to be handled separately. For example, after calling dlt_daemon_exit_trigger() set the SIG_DFL for SIGSEGV and raise() the same signal (or just let it hit SIGSEGV on its own again).

I really don't advise calling dlt_daemon_exit_trigger() since it calls async-unsafe functions in a signal handler, but since you already break that rule I wont complain.

{
/* finalize the server */
dlt_vlog(LOG_NOTICE, "Exiting DLT daemon due to signal: %s\n",
Expand Down
Loading