Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Qt Splash Screen is deleted (and accesses wallet) after Shutdown() (and wallets are deleted). #12372

Closed
TheBlueMatt opened this issue Feb 6, 2018 · 3 comments · Fixed by #12374
Closed

Qt Splash Screen is deleted (and accesses wallet) after Shutdown() (and wallets are deleted). #12372

TheBlueMatt opened this issue Feb 6, 2018 · 3 comments · Fixed by #12374
Labels
GUI
Milestone
0.16.0

Comments

@TheBlueMatt
Copy link
Contributor

In trying to debug #12337 I added a sleep at the end of AppInitMain() to simulate being able to close the application late in the AppInitMain() process. If I close the splash screen at this point, I get a reliable crash when the splashs screen is deleted as it tries to disconnect itself from the wallet ShowProgress signal after Shutdown() has been called and the wallet has been deleted.
@maflcko maflcko added this to the 0.16.0 milestone Feb 6, 2018
@maflcko maflcko added Wallet and removed Wallet labels Feb 6, 2018
@maflcko
Copy link
Member

maflcko commented Feb 7, 2018

@TheBlueMatt Are you sure this is something with the wallet?

I think the issue is that the shutdown process has started in requestShutdown, but initialize will happily continue with initializeResult and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

@maflcko
Copy link
Member

maflcko commented Feb 7, 2018

Potentially I am talking about a different issue.

@maflcko maflcko added the GUI label Feb 7, 2018
@laanwj laanwj mentioned this issue Feb 7, 2018
Merged
@laanwj
Copy link
Member

laanwj commented Feb 7, 2018

Should be fixed by #12374.

laanwj added a commit to laanwj/bitcoin that referenced this issue Feb 7, 2018
@laanwj
qt: Make sure splash screen is freed on AppInitMain fail
f5a4c3d 
The `splashFinished` event was never sent if AppInitMain fails,
causing the splash screen to stick around, causing problems
later.

This bug has existed for a while but is now trigging potential crashed
because the splash screen subscribes to wallet events.

Meant to fix bitcoin#12372.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
@maflcko maflcko mentioned this issue Feb 7, 2018
Merged
laanwj added a commit that referenced this issue Feb 8, 2018
@laanwj
Merge #12377: qt: Poll ShutdownTimer after init is done
36a927c 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for #12372 (comment)

  This reverts #11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
laanwj added a commit that referenced this issue Feb 8, 2018
@laanwj
Merge #12374: qt: Make sure splash screen is freed on AppInitMain fail
11f3eac 
1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)

Pull request description:

  The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.

  This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.

  Meant to fix #12372.

Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
laanwj added a commit that referenced this issue Feb 8, 2018
@laanwj
qt: Make sure splash screen is freed on AppInitMain fail
daaae36 
The `splashFinished` event was never sent if AppInitMain fails,
causing the splash screen to stick around, causing problems
later.

This bug has existed for a while but is now trigging potential crashed
because the splash screen subscribes to wallet events.

Meant to fix #12372.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
Github-Pull: #12374
Rebased-From: f5a4c3d
Tree-SHA512: 1c59633f0caec6344dce7f7d69d2e98242601fa906b1845c372a59c8ba015c3ac76389dd5d4e60b2fdb52d2878d566a0325679470075a680418cade7204069ef
hkjn pushed a commit to hkjn/bitcoin that referenced this issue Feb 12, 2018
@laanwj @hkjn
qt: Make sure splash screen is freed on AppInitMain fail
b3f378a 
The `splashFinished` event was never sent if AppInitMain fails,
causing the splash screen to stick around, causing problems
later.

This bug has existed for a while but is now trigging potential crashed
because the splash screen subscribes to wallet events.

Meant to fix bitcoin#12372.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this issue Mar 16, 2018
@laanwj
qt: Make sure splash screen is freed on AppInitMain fail
8cbba50 
The `splashFinished` event was never sent if AppInitMain fails,
causing the splash screen to stick around, causing problems
later.

This bug has existed for a while but is now trigging potential crashed
because the splash screen subscribes to wallet events.

Meant to fix bitcoin#12372.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
Github-Pull: bitcoin#12374
Rebased-From: f5a4c3d
Tree-SHA512: 1c59633f0caec6344dce7f7d69d2e98242601fa906b1845c372a59c8ba015c3ac76389dd5d4e60b2fdb52d2878d566a0325679470075a680418cade7204069ef
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
6b32a15 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12374: qt: Make sure splash screen is freed on AppInitM…
5ed53ff 
…ain fail

1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)

Pull request description:

  The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.

  This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.

  Meant to fix bitcoin#12372.

Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
bf1d2ce 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12374: qt: Make sure splash screen is freed on AppInitM…
61bd9bd 
…ain fail

1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)

Pull request description:

  The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.

  This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.

  Meant to fix bitcoin#12372.

Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 15, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
7e5c5d4 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 15, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12374: qt: Make sure splash screen is freed on AppInitM…
aa34483 
…ain fail

1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)

Pull request description:

  The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.

  This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.

  Meant to fix bitcoin#12372.

Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
gades pushed a commit to cosanta/cosanta-core that referenced this issue Mar 10, 2022
@laanwj @gades
Merge bitcoin#12374: qt: Make sure splash screen is freed on AppInitM…
caaa5ca 
…ain fail

1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)

Pull request description:

  The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.

  This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.

  Meant to fix bitcoin#12372.

Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
GUI
Projects
None yet
Milestone
0.16.0
Development

Successfully merging a pull request may close this issue.

qt: Make sure splash screen is freed on AppInitMain fail laanwj/bitcoin
3 participants
@laanwj @TheBlueMatt @maflcko