splintered-reality/py_trees_ros

[0.5.x][Blackboard][View] crash on non-pickleable objects in blackboard

Closed this issue · 5 comments

When publishing the blackboard or watching non-pickleable objects through py-trees-blackboard-watcher, the tree would crash. This is because of exception in dumps.

example:

>>> obj = rospy.Publisher('foo', std_msgs.Empty, queue_size=1)
>>> dumps(obj, -1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't pickle thread.lock objects

One some other objects it's raising cPickle.PicklingError.

It might not be a good practice to put classes in the blackboard but in our code base, few of them are there and this happened when somebody hit rostopic echo /tree/blackboard (actually it was rosbag record --all)

In the devel version, I see pickle being used instead of cPickle, but it seems to have the same problem.

I just had a pickle conversation with someone last week and learned of the fragility of pickle. Surprised I hadn't run into this problem myself for so long.

In general, I've liked the ability to store classes on the board (saves many lines of extra code) and even taken care to provide accessors that get directly at the attributes (e.g. self.blackboard.foo_object.nested_bar_attribute).

What have you been doing to workaround the problem? Just not storing the entire object?

Things that might be worth doing...

  • Catch the exceptions and provide a meaningful warning, don't crash the tree (not publishing the blackboard doesn't prevent the tree from being able to continue to execute).
  • Check if the object is pickleable on setting the object? Can't do in 0.5.x, but could do so in the recent blackboard upgrade. Could provide early warnings and cause the variable to be skipped (special snowflake clause) in the pickling check
  • A data format with structure (... JSON) that would permit easier nested access / tracking on reads / pickling without problems, but require users to convert their data before writing
  • ...

To us it's not a problem, because we are never subscribing to blackboard or watching those particular objects. So those objects can sit there (or we redesign the software to reduce the abuse of blackboard)

Atleast in 0.5, I'd be favour of just 'skipping' pickling if not pickleable as opposed to adding picklifying helpers cause it adds more things to take care of (folks here have yet to grasp pre/post tick, visitors etc) and many classes would be picklifyable (so self.blackboard.foo_object.nested_bar_attribute remains valid).

I think we can skip those keys which are not pickle-able when dumping, somethink like:

  • keep track of objects that are not pickleable, and whenever something is added to bb we check and add
  • on publish/watch, those keys are subtracted from the dictionary before dumping.

Subtracting them does potentially mean you'll miss updates because they won't be in the check for changes in the blackboard. To check on adding I'd have to re-implement __setattr__ in v0.5 (I do this now in v1.3), which might be just a three line addition, but might have fallout as well.

How about just checking for a pickle error on dumping in v0.5, send a throttled warning and publish regardless. This will have the downside of suboptimal publishing even when there isn't change, but at least you won't miss changes. Once people see the warning, they can aim to be more frugal in dumping only certain attributes of the problem object on the board rather than the unpicklable whole.

I'll work on a better solution in v1.3.

I am interested in a solution for this as well, need to stick with 0.5 because we are using Melodic.

There is some information out there about this problem but you need to recursively check all the objects which could be slow. In my case it is an array of ros messages that it can't handle.

Releasing in 2.0.11 and 0.5.19. Please re-open if you have further problems with this.