robust-rosin / robust

A dataset of 200+ bugs in the Robot Operating System for BugZoo
30 stars 10 forks source link

ros_comm/ca23e58: crash or corruption? #417

Closed gavanderhoorn closed 3 years ago

gavanderhoorn commented 3 years ago

Looking into the behavioural theme, I noticed ros_comm/ca23e58 is marked as interesting.

Whether it would fit the behavioural theme or not depends a bit on whether we expect the unsafe double-checked locking identified by @git-afsantos in https://github.com/ros/ros_comm/issues/770 is more likely to result in crashes or corruption / undefined behaviour.

(for crashes we have the Crashing theme)

@git-afsantos: would you remember?

git-afsantos commented 3 years ago

I'm not familiar enough with how TopicManager is used, nor with C++03 object constructions specifics. But re-reading the issue, and assuming it works more or less like Java (does or used to), I am more inclined towards crash.

Example scenario:

  1. Thread A calls TopicManager::instance(); it is not initialized, and the thread enters the mutex.
  2. Thread A allocates memory and assigns TopicManagerPtr topic_manager (object not constructed).
  3. Thread B calls TopicManager::instance() and sees it assigned.
  4. Thread B calls, e.g., TopicManager::start(), which tries to access the shutting_down_mutex_ field.
  5. The field is not initialized, and thus contains bogus memory; it is not a valid memory address, Thread B crashes (segmentation fault, maybe?).
gavanderhoorn commented 3 years ago

Yes, I agree, your analysis makes sense.

I'm going to move it to the Crashing theme.