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
[humble] Bugfix for writer not being able to open again after closing (backport #1599) #1653
Conversation
Cherry-pick of a360d9b has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
…backport #1599) (#1635) * 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> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * 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> * Address build failure by using rcpputils::fs instead of std::filesystem - Note: On Iron we haven't migrated to the std::filesystem and using rcpputils::fs Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Adopt failing 'open_succeeds_twice' test for Iron Signed-off-by: Michael Orlov <michael.orlov@apex.ai> * Return from writer's open() immediately if storage already open 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: Yannick Schulz <yschulz854@gmail.com> (cherry picked from commit a360d9b) # Conflicts: # rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp # rosbag2_cpp/src/rosbag2_cpp/writer.cpp # rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
- Ensure that writer_ is destructed before intercepted fake_metadata_ Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
- Use std::filesystem for temp files and folders operation. For some reason rcpputils::fs::delete_all(folder_name) wasn't able to delete temp folder with subfolders. Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
8c80e55
to
e0d3d48
Compare
Pulls: #1653 |
- The `serialized_msg2` is not owning the serialized data after the first call writer.write(serialized_msg2,..). i.e. need to use another message or another API in test. This is not a bug - this is by design. Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Re-run CI after addressing test failure. |
Note: There are no final changes in header files. i.e., ABI/API compatible.
This is an automatic backport of pull request #1635 done by Mergify.