8000 [GUI] Do not use Qt's debugging information in the log handler. by CaveSpectre11 · Pull Request #774 · Veil-Project/veil · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[GUI] Do not use Qt's debugging information in the log handler. #774

New is 8000 sue

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

Merged
merged 1 commit into from
Jun 20, 2020

Conversation

CaveSpectre11
Copy link
Collaborator

(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.

* This information is not always available and caused crashes
    in some builds.
@CaveSpectre11 CaveSpectre11 added Priority: 3 - High Critical item that should be done as soon as possible Tag: Segfault This issue causes the application to crash. Tag: OS - OSX Problems specific to Mac OS labels Apr 17, 2020
@CaveSpectre11 CaveSpectre11 changed the title Do not use Qt's debugging information in the log handler. [GUI] Do not use Qt's debugging information in the log handler. Apr 17, 2020
Copy link
Collaborator
@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 5416ae0

Copy link
Contributor
@mimirmim mimirmim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously can't build it right now but clear enough to get moving forward on this.

ACK (but fails build).

@philmb3487
Copy link
Contributor

ack

@codeofalltrades codeofalltrades added the Tag: Waiting For QA A pull review is waiting for QA to test the pull request label May 2, 2020
@CaveSpectre11 CaveSpectre11 added QA: Passed This has passed QA testing and can be merged to master and removed Tag: Waiting For QA A pull review is waiting for QA to test the pull request labels Jun 20, 2020
@CaveSpectre11 CaveSpectre11 merged commit b90f387 into Veil-Project:master Jun 20, 2020
@CaveSpectre11 CaveSpectre11 added the Dev Status: Merged Issue is completely finished. label Jun 20, 2020
@CaveSpectre11 CaveSpectre11 deleted the mac-debug-fix branch July 13, 2020 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Review: Passed Dev Status: Merged Issue is completely finished. Priority: 3 - High Critical item that should be done as soon as possible QA: Passed This has passed QA testing and can be merged to master Tag: OS - OSX Problems specific to Mac OS Tag: Segfault This issue causes the application to crash.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0