SteveMacenski
commented
2 months ago Seems like we should mutex lock the database reload service from being callable while the action is actively running
I think the best behavior is to use the original values until the action finishes, so I would make a copy that doesn't get destroyed on reload.
That may work too! These are small data structures, so a copy would also be fine. My main concern here is that if you call DockID = 7 and change what DockID = 7 means during docking with 7, someone complains that it didn't auto-fine the new database values. So that's what makes me think it would also be reasonable to set the behavior that updating the dock poses during docking is inappropriate and if we can't lock some mutex, return response->success = false; to the service call that it cannot be processed simultaneously.
If the reload_database service is called during docking, the dock pointers get invalidated and the docking_server crashes.
It crashes specifically on this line. https://github.com/open-navigation/opennav_docking/blob/4a63900ca8d0609fe0663d8b87edb4d9f5a0561f/opennav_docking/src/docking_server.cpp#L367
I think the best behavior is to use the original values until the action finishes, so I would make a copy that doesn't get destroyed on reload. Looks like an easy fix, I can open a PR but I wanted to hear your thoughts about this @SteveMacenski.