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.

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

Conversation

duvanan13
Copy link
Contributor

  • Prevent new daemon initialization when using FIFO
  • Unit test gateway IPC changes to UNIX_SOCKET
  • Shared memory is used for multinode testing
  • Resolve the TOCTOU issue

+ Prevent new daemon initialization when using FIFO
+ Unit test gateway IPC changes to UNIX_SOCKET
+ Shared memory is used for multinode testing
+ Resolve the TOCTOU issue

Signed-off-by: andv <[email protected]>
Signed-off-by: LUU QUANG MINH <[email protected]>
@minminlittleshrimp
Copy link
Collaborator

Thanks @duvanan13 for joining the pair programming section.
I believe now you have more experience with source control using git.
Thank you for your the rework on this issue, let see how Michael comment about this fix.

Hello @michael-methner , could you kindly review this fix?
Thank you

@michael-methner
Copy link
Collaborator

Hello @duvanan13 , hello @minminlittleshrimp ,
this patch prevents are restart after dlt-daemon was not shutdown gracefully (e.g. due to a segmentation fault). Could modify the check so it checks for a running instance of dlt-daemon instead of the files present?

@duvanan13
Copy link
Contributor Author

duvanan13 commented Jul 17, 2024

Hello @michael-methner ,
In the dlt_daemon_signal_handler function in daemon, we've already implemented the unlink for FIFO by calling dlt_daemon_exit_trigger. Hence, my point is to add SIGSEGV to the dlt_daemon_signal_handler:

(void)unlink(tmp);

Please kindly check my point.

@@ -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)

@@ -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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Another dlt-dameon recreate fifo file before exit error
4 participants