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

gazebo and gzserver return without error if gzserver is already running #1178

Closed
osrf-migration opened this issue May 6, 2014 · 10 comments
Closed
Labels
all bug Something isn't working major

Comments

@osrf-migration
Copy link

Original report (archived issue) by andy_somerville (Bitbucket: andy_somerville).


If a gzserver process is already running gazebo and gzserver return without error. This is bad in that systems which pass arguments and expect respective action, systems which expect gzclient to come up when gazebo is run, or systems which expect the call to block until finish do not behave correctly.

Specifically roslaunch drcsim_gazebo atlas.launch does not behave correctly when gzserver is already running.

Optimal behavior is unclear, but a message to stderr seems prudent.

From the perspective of gzclient expectations:
gazebo typically brings up both server and client when there isn't a server running. If a server is already running it seems that it should either bring up the client, or return an error.

From the perspective of plugin loading, it's unclear if the plugins are in-fact loaded if gzserver is already running. If loading does not happen, gazebo should return an error since post conditions aren't meant.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


The check for an existing server is in void Master::Init, which throws an exception if detected. We could change the return type to bool Master::Init and return false instead of throwing an exception. This would break ABI.

Adding different logic to the gazebo startup would take a little more work.

@osrf-migration
Copy link
Author

Original comment by andy_somerville (Bitbucket: andy_somerville).


I think the important change would be in what is eventually returned to the system rather than what is returned internally.

It seem the problem would be more in server_main.cc and gazebo_main.cc in that server_main catches all exceptions and without changing the return value to the system:

#!c++

int main(int argc, char **argv)
{
  gazebo::Server *server = NULL;

  try
  {
   ...
  }
  catch(gazebo::common::Exception &_e)
  {
    _e.Print();

    server->Fini();
    delete server;
  }

  return 0;
}

I think adjusting the catch to change the return value to -1 or some other specific error code could be part of a solution which would not break ABI. Though it is unclear if there are any users currently depending on the system to return a 0 even if a server is already running.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I should have mentioned that Master::Init is called by bool gazebo::setupServer, which traces back to server_main.cc through Server::Preload and Server::ParseArgs. So, I think it would propagate back to the return code.

@osrf-migration
Copy link
Author

Original comment by andy_somerville (Bitbucket: andy_somerville).


I think that increases the number of options without needing to change the ABI.

It would mean that any one of those in that line could catch the exception and turn it into a return code. gazebo::setupServer being more or less the first line of defense could catch that exception and return false.

In any case it seems like the final catch in any main should probably interpret an exception as an error and return non-zero accordingly.

I can produce a patch/pull request for this, but unfortunately I'm not set up to easily build and test which might reduce the utility of me doing so.

@osrf-migration
Copy link
Author

Original comment by Jose Luis Rivero (Bitbucket: Jose Luis Rivero, GitHub: j-rivero).


  • set assignee_account_id to "557058:155a32e2-420c-4d50-98e0-0e722f63f906"
  • set assignee to "jrivero (Bitbucket: jrivero, GitHub: j-rivero)"

I'm interested in this problem, I'll try to create a patch from Andy's comments. Let's see if can keep the ABI stable.

@osrf-migration
Copy link
Author

Original comment by Jose Luis Rivero (Bitbucket: Jose Luis Rivero, GitHub: j-rivero).


The pull request #1085 should implement proper returning codes for gazebo 2.2 branch.

About the message for another gzserver instance running, in gazebo3 (or default) I was able to see clearly the message in ~/ .gazebo/gzserver.log. For gazebo 2.2, the message is directly sent to the terminal. Both cases seems well documented to me.

@osrf-migration
Copy link
Author

Original comment by andy_somerville (Bitbucket: andy_somerville).


The pull request #1085 should implement proper returning codes for gazebo 2.2 branch.

Awsome!

gazebo3 (or default) I was able to see clearly the message in ~/ .gazebo/gzserver.log.

I can verify that I also see the error message there. ~/.gazebo/gzserver.log was not a place I checked probably because I'm used to only checking the ROS logs which did not have said message, but that's more of a ROS specific problem which exists for all packages where ROS is not a direct dependency.

That said, is there any reason errors don't also show up on stderr for gazebo3?

@osrf-migration
Copy link
Author

Original comment by Jose Luis Rivero (Bitbucket: Jose Luis Rivero, GitHub: j-rivero).


  • changed state from "new" to "resolved"

pull request merged. Reopen if needed.

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • set version to "all"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "resolved" to "closed"

@osrf-migration osrf-migration added major bug Something isn't working all labels Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

1 participant