-
Notifications
You must be signed in to change notification settings - Fork 141
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
Serialization failure with MSVC and version 1.84 #309
Comments
I looked at the links cited. I'm not sure how to understand the build script and how/if it points to the boost serialization library. |
This is the test that is failing: https://github.com/tesseract-robotics/tesseract/blob/master/tesseract_environment/test/tesseract_environment_serialization_unit.cpp This contains the test utility functions that implement serialization: https://github.com/tesseract-robotics/tesseract/blob/master/tesseract_common/include/tesseract_common/unit_test_utils.h |
im currently upgrading to 1.84 - so just commenting here to be informed by github if there is a real issue |
Note that the serialization to string and XML works correctly so I don't think the issue is in the tesseract library. |
I did more tinkering and was able to find this difference between the failing unit test and an isolated example that seems to work correctly.
The serialization step is here: https://github.com/johnwason/tesseract/blob/51a62b5bf3d844f8605b76879ac0f4c38b7d059b/tesseract_environment/src/commands/change_joint_position_limits_command.cpp#L81 and the class is defined here: https://github.com/johnwason/tesseract/blob/51a62b5bf3d844f8605b76879ac0f4c38b7d059b/tesseract_environment/include/tesseract_environment/commands/change_joint_position_limits_command.h#L45 Maybe someone with more knowledge of the serialization format can explain what could cause this difference. |
I have been able to isolate and reproduce the problem. Here is the repo with the test code: https://github.com/johnwason/boost_serialization_1_84_bug And here is the action run: https://github.com/johnwason/boost_serialization_1_84_bug/actions/runs/8745401644/job/24000327991 What I have discovered is that if the classes being serialized is in a shared library, then the serialization will fail. The failure only occurs for binary archives. The action run tests version 1.83 and version 1.82 of boost serialization, and it appears to work correctly. Here is the cmake file. The tests for test_no_lib_prog works, while test_lib_prog fails. cmake_minimum_required(VERSION 3.15.0)
project(boost_serialization_1_84_bug LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 17)
find_package(Boost REQUIRED COMPONENTS system filesystem serialization)
find_package(GTest REQUIRED)
add_library(test_lib SHARED cmd.cpp cmd2.cpp)
target_link_libraries(test_lib Boost::boost
Boost::system
Boost::filesystem
Boost::serialization
GTest::GTest GTest::Main)
set_target_properties(test_lib PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
add_executable(test_lib_prog test.cpp)
target_link_libraries(test_lib_prog test_lib Boost::boost
Boost::system
Boost::filesystem
Boost::serialization
GTest::GTest GTest::Main)
add_executable(test_no_lib_prog test.cpp cmd.cpp cmd2.cpp)
target_link_libraries(test_no_lib_prog Boost::boost
Boost::system
Boost::filesystem
Boost::serialization
GTest::GTest GTest::Main)
enable_testing()
add_test(NAME test_no_lib COMMAND test_no_lib_prog)
add_test(NAME test_lib COMMAND test_lib_prog) Here is the ctest log:
|
I am still seeing this failure in boost 1.85. |
Are there any suggestions where I can begin troubleshooting? |
I can’t help you, I don’t have experience with Windows. but I am curious what you do mean by “classes being serialized is in a shared library”? do you mean that especial members are linked in a dynamic library? dll? |
Sorry I haven't been more responsive. As far as Boost is concerned, I've been bogged down in build/test issues which have nothing to do with the serialization library itself. |
You've done some real work here. I would agree that your experiments with different archives suggest that it's an issue with binary archive suggest that the problem lies there. That narrows things down a lot. Could you send the source code to your tests - both the failing and passing one? |
The source code for the isolated test is here: https://github.com/johnwason/boost_serialization_1_84_bug . It includes an actions workflow that does a parametric test of different boost versions. The original problem was discovered in the Tesseract robot motion planner: https://github.com/tesseract-robotics/tesseract . The isolated test was extracted from this source. |
Excuse my ignorance, but what is the meaning of "&=" for an Archive in cmd.cpp (line 15 for example) ? |
LOL - good question! I'd be curious to hear the answer as well. |
It looks like it is a typo. There are 656 lines of serialization code within Tesseract and only two lines of code has the syntax |
@robertramey and @correaa Any thoughts why this syntax works at all? |
I'm mystified by this. I did look into it but didn't see wow it would not produce a syntax error. I'm going to run your test case with the debugger. I expect this will address the error you found and also shed light on this. |
I changed I am getting the warning "warning C4552: '&': result of expression not used" now. Can this be ignored, or is there something else we are missing? I am going to test the main tesseract library to see if the test pass. |
I guess that &= was converting either side of the expression to |
My isolated test case is fixed, but I am still getting crashes and failures in the main library. I am only seeing this issue for the binary archive so I don't think it is something in the client code. |
Isolate the error again? |
As I've said, I don't see the operator &=defined for an archive type - so I would expect a syntax error. |
It looks like I nerfed the test rather than fixing the error. Here is the corrected run showing that it is still failing: https://github.com/johnwason/boost_serialization_1_84_bug/actions/runs/9458596594 |
One possibility I noticed is that there is a singleton template that is being initialized with separate static values in the different libraries. This would mean that the base class is seeing a different singleton than the derived class when trying to serialize. Was something involving singletons changed in the more recent versions? |
@robertramey and @correaa This is still failing after the fix. |
I found the other issue. The cpp was missing |
The fix by @Levi-Armstrong fixed the test case. I am working on the full library test now. ... Why did this work before for other boost versions and different OS?? |
This appears to be fixed with tesseract-robotics/tesseract#1035 Closing the issue. |
We are seeing a new issue with serialization using MSVC and version 1.84. Unit tests are failing, and it looks like the serialization is being corrupted with binary archives, and some tests are failing with XML archives. The serialization has worked with previous versions of serialization. I did a matrix test using version 1.82, 1.83, and 1.84 to confirm. 1.82 and 1.83 passed, while 1.84 failed. Here is the workflow run:
https://github.com/johnwason/tesseract/actions/runs/8242279639
The upstream source repository is here: https://github.com/tesseract-robotics/tesseract
Both tesseract and boost-serialization are large projects so I am not sure where the problem is. Were there any major changes in boost-serialization that would explain the problem?
@Levi-Armstrong
The text was updated successfully, but these errors were encountered: