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

Jump to bottom

Does lock necessary for parameter server access? #401

Closed
jihoonl opened this issue May 7, 2014 · 6 comments
Closed

Does lock necessary for parameter server access? #401

jihoonl opened this issue May 7, 2014 · 6 comments
Labels
invalid

Comments

@jihoonl
Copy link

jihoonl commented May 7, 2014

I wonder if parameter server is not thread-safe anymore. I have tried to get and set params almost simultaneously using rosbridge, and it raised an error.

Here are some test code using rosbridge. RobotWebTools/rosbridge_suite#103.

I believe this is introduced recently since I was able to replicate the problem using default roslibjs test code. The test code was developed more than a year ago and worked fine until I see this issue. And I can confirm that no logic inside of rosbridge has changed.
@dirk-thomas
Copy link
Member

With which version of ros_comm are you experiencing the issue? Can you provide a reproducible example to ease our troubleshooting?

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

I am testing with the hydro release version of ros_comm 1.10.2.

Run the deb version of rosbridge and open up the webpage below. You might want to wget roslibjs and eventemitter2 beforehand.

> roslaunch rosbridge_server rosbridge_websocket.launch --screen

# where the webpage get served
> wget https://raw.githubusercontent.com/RobotWebTools/roslibjs/devel/include/EventEmitter2/eventemitter2.js
> wget https://raw.githubusercontent.com/RobotWebTools/roslibjs/devel/build/roslib.js

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
<!--<script src="http://cdn.robotwebtools.org/EventEmitter2/current/eventemitter2.min.js"></script>-->
<script src="eventemitter2.js"></script>
<script src="roslib.js"></script>

<script>


var ros = new ROSLIB.Ros();

  // If there is an error on the backend, an 'error' emit will be emitted.
  ros.on('error', function(error) {
          console.log(error);
            });

  // Find out exactly when we made a connection.
  ros.on('connection', function() {
          console.log('Connection made!');

            ros.getParams(function(params) {
                  console.log(params);
                    });

              // First, we create a Param object with the name of the param.
              var maxVelX = new ROSLIB.Param({
                    ros : ros,
                        name : 'max_vel_y'
                          });

                //Then we set the value of the param, which is sent to the ROS Parameter Server.
                maxVelX.set(0.8);
                  });

  // Create a connection to the rosbridge WebSocket server.
  ros.connect('ws://localhost:9090');
  </script>
</head>

<body>
Test
</body>
</html>

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

If you refresh the webpage a couple of times you will see something like this. Since it is thread related , I couldn't find any better way to replicate the problem...

[INFO] [WallTime: 1399427968.896866] Client connected.  1 clients total.
[ERROR] [WallTime: 1399427968.905601] [Client 0] [id: call_service:/rosapi/set_param:2] call_service InvalidPackageException: Unable to load the manifest for package rosapi. Caused by: rosapi
[INFO] [WallTime: 1399427969.766790] Client disconnected. 0 clients total.
[INFO] [WallTime: 1399427969.781902] Client connected.  1 clients total.
Traceback (most recent call last):
  File "/opt/ros/hydro/lib/python2.7/dist-packages/rospy/impl/tcpros_service.py", line 618, in _handle_request
    response = convert_return_to_response(self.handler(request), self.response_class)
  File "/opt/ros/hydro/lib/rosapi/rosapi", line 135, in set_param
    rosapi.params.set_param(request.name, request.value)
  File "/opt/ros/hydro/lib/python2.7/dist-packages/rosapi/params.py", line 49, in set_param
    rospy.set_param(name, d)
  File "/opt/ros/hydro/lib/python2.7/dist-packages/rospy/client.py", line 494, in set_param
    _param_server[param_name] = param_value #MasterProxy does all the magic for us
  File "/opt/ros/hydro/lib/python2.7/dist-packages/rospy/msproxy.py", line 140, in __setitem__
    self.target.setParam(rospy.names.get_caller_id(), rospy.names.resolve_name(key), val)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1224, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1578, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.7/xmlrpclib.py", line 1264, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1289, in single_request
    self.send_request(h, handler, request_body)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1391, in send_request
    connection.putrequest("POST", handler, skip_accept_encoding=True)
  File "/usr/lib/python2.7/httplib.py", line 856, in putrequest
    raise CannotSendRequest()
CannotSendRequest
[ERROR] [WallTime: 1399427969.790138] Error processing request: 
None
[ERROR] [WallTime: 1399427969.790464] [Client 1] [id: call_service:/rosapi/set_param:2] call_service ServiceException: service [/rosapi/set_param] responded with an error: error processing request:

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

The actuall set/get param is called in rosapi.

https://github.com/RobotWebTools/rosbridge_suite/blob/develop/rosapi/src/rosapi/params.py#L43-L67

@dirk-thomas
Copy link
Member

I can see the same error messages but for me it happens on every refresh - not only occasionally. I am not sure if the stacktrace is related to multi threaded invocation.

But the Python parameter API does not provide thread-safe access (http://wiki.ros.org/rospy/Overview/Parameter%20Server). You might need to deal with the locking in your user land code. Perhaps you want to fill this ticket against rosbridge so that deal with locking for you?

@jihoonl
Copy link
Author

jihoonl commented May 7, 2014

Ok. Actually, @lrasinen has created a PR to patch rosbridge side. But I just wanted to make sure if anything happen in ros_comm.

Because, I have noticed similar behavior when I tried to call roslib.launcher.load_manifest while testing this question. And also I thought it is a new behavior which didn't happen before since roslibjs test code has been around for a year without any problem.

I will proceed the patch in rosbridge side. Thanks for explanation @dirk-thomas

@jihoonl jihoonl closed this as completed May 7, 2014
@jihoonl jihoonl mentioned this issue May 7, 2014
Merged
This was referenced Sep 25, 2014
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
invalid
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dirk-thomas @jihoonl