Skip to content

Conversation

@XINJIANGMO
Copy link
Contributor

🦟 Bug fix

Fixes #3132

Summary

In the previous flow, the GUI thread (which spawns a separate GUI subprocess) starts before createServerConfig runs. So when gz.cc prints the error, the GUI is already launched; returning or calling std::exit in gz.cc won’t stop the GUI subprocess started via std::system.
This order also causes the crash: main exits while the GUI std::thread remains joinable, its destructor calls std::terminate, resulting in terminate called without an active exception.

This fix adds a check for this invalid combo in sim_main.cc to avoid thread issue.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Copy link
Contributor

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Thanks for catching this, it makes sense. LGTM

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Oct 18, 2025
// check for invalid combination to avoid thread issue
if (!opt->file.empty() && !opt->playback.empty())
{
gzerr << "Both an SDF file and the playback flag (--playback) are specified. "
Copy link
Contributor

@iche033 iche033 Oct 31, 2025

Choose a reason for hiding this comment

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

minor issue picked up by CI:

/github/workspace/src/cmd/sim_main.cc:467:  Lines should be <= 80 characters long 

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

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

3 participants