Prevent segfault in casacore log sink destructor #1398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… pointer it's not safe to reuse
pford
approved these changes
Jan 28, 2025
pford
reviewed
Jan 29, 2025
src/Main/Main.cc
Outdated
| casacore::LogSink::globalSink(carta_log_sink); | ||
| casacore::LogIO casacore_log(log_sink); | ||
| casacore::LogIO casacore_log; |
There was a problem hiding this comment.
Good point; this object is actually never used.
|
kswang1029
approved these changes
Feb 8, 2025
kswang1029
added
awaiting merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This fixes #1382. Testing is required to check if this is still doing the correct thing with casacore logs.
The segfault happens in the shared pointer destructor called from the
casacore::LogSinkdestructor. I believe that this is because the shared pointer's underlying pointer is zeroed bycasacore::LogSink::globalSinkwhen it is used to set the global sink. This PR bypasses this issue by not reusing the same raw pointer both to set the global sink and to create aLogSinkobject which theLogIOobject is then attached to. Instead, it uses the defaultLogIOconstructor, which should attach the object to the global sink, which should be set to the correct sink.I tested this locally and it appears that the casacore messages are still being formatted correctly in the output, but I'm not 100% familiar with the way that this logging model is supposed to work.
If there is no regression, I think that this PR is low-risk enough to be included in the freeze, but I'm creating it as a draft first just in case.
Checklist
no changelog update neededprotobuf updated to the latest dev commit/ no protobuf update neededprotobuf version bumped/ protobuf version not bumped