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

remove receive signal handler #641

Merged
merged 1 commit into from
Jan 18, 2025
Merged

remove receive signal handler #641

merged 1 commit into from
Jan 18, 2025

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Jan 18, 2025

The only difference when receiving a termination signal is this line not being ran, but the program would be exiting anyway if this is the case:

msgq_msg_close(&msg); // Free unused message on exit

With the handler, the polling is not interrupted, so we hang until the timeout is up. If this is set high, we can get stuck here on exit.

@sshane sshane changed the title remove signal handler remove receive signal handler Jan 18, 2025
@sshane sshane merged commit 102befe into master Jan 18, 2025
16 checks passed
@sshane sshane deleted the no-signal-handler branch January 18, 2025 04:33
@deanlee
Copy link
Contributor

deanlee commented Jan 18, 2025

I like this solution; it resolves the issue while reducing complexity.

@sshane
Copy link
Contributor Author

sshane commented Jan 18, 2025

Something about this is causing pandad to hang, will need to check why

@deanlee
Copy link
Contributor

deanlee commented Jan 18, 2025

It may not be directly related to the pandad issue, but we should removing the if (errno == EINTR) check in pandad after this PR.

  // run as fast as messages come in
  while (!do_exit && check_all_connected(pandas)) {
    std::unique_ptr<Message> msg(subscriber->receive());
    if (!msg) {
      if (errno ==   EINTR) {
        do_exit = true;
      }
      continue;
    }

MarcoTheDingo added a commit to MarcoTheDingo/msgq that referenced this pull request Jan 19, 2025
MarcoTheDingo added a commit to MarcoTheDingo/msgq that referenced this pull request Jan 19, 2025
@sshane
Copy link
Contributor Author

sshane commented Jan 22, 2025

thanks, that was it!

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

Successfully merging this pull request may close these issues.

2 participants