Address segmentation faults in the Log message handlers on macOS. #766
Conversation
Qt `setWindowIcon()` does this work.
This moves the Dock icon click reaction code to the common place and allows some cleanup in obj_c code. According to the Apple's docs `class_replaceMethod` behaves as `class_addMethod`, if the method identified by name does not yet exist; or as `method_setImplementation`, if it does exist.
Qt `setAsDockMenu()` does this work.
|
@CaveSpectre11 |
There was a problem hiding this comment.
@sinetek thank you for using the form. You don't actually need to include the bounty and payment address sections. I had put those in when I developed the form since I was a bounty dev at the time. You can just remove those; no problem.
So I added the problem code when I was doing some Qt fixes recently. I wrapped with the #ifndef to prevent the new code from being used when the relevant QT_MESSAGELOGCONTEXT wasn't defined. Do you know where it's getting defined from?
I left the code in so that it could be used without people having to figure out how to add it. With that said, I would prefer that it be commented out, rather than removed from the code.
Great find!
CaveSpectre11
added
Tag: Build System
Tag: OS - OSX
|
ACK This does fix the segfault when natively compiling on OSX. I have not yet tried to compile using gitian, but I will tomorrow |
* This information is not always available and caused crashes
in some builds.
|
@CaveSpectre11 Changes done. |
philmb3487
added
the
Priority: 3 - High
CaveSpectre11
added
Tag: Waiting For Code Review
CaveSpectre11
removed
the
Priority: 3 - High
…ndler. 5416ae0 Do not use Qt's debugging information in the log handler. (sinetek) Pull request description: (Commit cherry picked from #766 for fast tracking) ### Problem ### CaveSpectre and mimir (others?) have reported a crash on recent macOS builds. The crash is a segmentation fault in a debug handler named DebugMessageHandler(), at runtime, before the slash screen, or main window, is shown. ### Root Cause ### The issue has been tracked down to incorrect usage of the debug.log message handler override. This is a part of Qt that allows us to log to debug.log, using a bypassed message handler in the form of a callback function. The issue is that we should not access some of the “context” information, passed in the form of a QmessageLogContext. If we peruse the Qt documentation: Note: By default, this information is recorded only in debug builds. You can overwrite this explicitly by defining QT_MESSAGELOGCONTEXT or QT_NO_MESSAGELOGCONTEXT. ### Solution ### The correct way to fix this is to not make use of any of the potentially unavailable debugging information. ### Unit Testing Results ### Build on a relatively recent version of macOS, and see if: 1) The build is successful. 2) The user succeeds in launching the application to the main window. Tree-SHA512: 7d15d01ecd406ba2738becd9d477a78aaa69babe77557927b6a8d4fd7535822b12c2ee38a3868946b8a41353ffbee2a5e413d631f350f523eba65cbc6138bf5e
|
Includes 3 of 6 commits from: bitcoin/bitcoin#14123 |
seanPhill
added
the
QA: In Progress
Problem
CaveSpectre and mimir (others?) have reported a crash on recent macOS builds.
The crash is a segmentation fault in a debug handler named DebugMessageHandler(),
at runtime, before the slash screen, or main window, is shown.
Root Cause
The issue has been tracked down to incorrect usage of the debug.log message handler override.
This is a part of Qt that allows us to log to debug.log, using a bypassed message handler
in the form of a callback function.
The issue is that we should not access some of the “context” information, passed in the form of
a QmessageLogContext.
If we peruse the Qt documentation:
Note: By default, this information is recorded only in debug builds. You can overwrite this explicitly by defining QT_MESSAGELOGCONTEXT or QT_NO_MESSAGELOGCONTEXT.
Solution
The correct way to fix this is to not make use of any of the potentially unavailable debugging information.
Furthermore, this Pull Request will pull in 4 additional commits, that are cherry-picks from Bitcoin upstream. The reason for this is to fix a build-time error on recent macOS versions,
due to code deprecation in the dock icon logic.
Unit Testing Results
Build on a relatively recent version of macOS, and see if: