-
Notifications
You must be signed in to change notification settings - Fork 240
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
[jazzy] Bugfix for writer not being able to open again after closing (backport #1599) #1639
Conversation
* re-applies fixes from #1590 to rolling. Also removes new message definition in sequential writer test for multiple open operations. Also clears topic_names_to_message_definitions_ and handles message_definitions_s underlying container similarly. Lastly, also avoids reset of factory in the compression writer, adds unit test there too. Signed-off-by: Yannick Schulz <yschulz854@gmail.com> * removes unused compressor_ member from compresser writer class. Also delegates rest of the closing behavior to the base class in close method, as it is handled in the open and write methods of the compression writer Signed-off-by: Yannick Schulz <yschulz854@gmail.com> * Remove unrelated delta - message_definitions_ was intentionally allocated on the stack and should persist between writer close() and open() because it represents cache for message definitions which is not changes. Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Don't call virtual methods from destructors Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Cleanup 'rosbag2_directory_next' after the test run Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Protect Writer::open(..) and Writer::close() with mutex on upper level - Rationale: To avoid race conditions if open(..) and close() could be ever be called from different threads. Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Bugfix for WRITE_SPLIT callback not called for the last compressed file Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Bugfix for lost messages from cache when closing compression writer Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Use dedicated is_open_ flag instead of relying on storage_ Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Yannick Schulz <yschulz854@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <michael.orlov@apex.ai> (cherry picked from commit 20cfc02)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clalancette Don't you mind to merge this bugfix to Jazzy?
It has ABI incompatible changes by adding a new private member
std::atomic_bool is_open_{false};
to the rosbag2_cpp::SequantialWriter
class.
@marcoag Are you ok with merging this bugfix to Jazzy now? |
Since it's a bugfix I'm ok with the merge into |
Pulls: #1639 |
rolling
. To reiterate, thestorage_factory
needs to persist beyond the call to close a current bag. On the other hand, all containers that were built during the recording have to be cleared before opening another bag.This is an automatic backport of pull request Bugfix for writer not being able to open again after closing #1599 done by Mergify.