gui: Set progressDialog to nullptr #16135
Merged
Conversation
This file contains hidden or 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
Contributor
Author
|
To backport. |
|
NACK I think you missed the one in WalletView::showProgress here: Line 318 in cd42553 |
Contributor
Author
promag
changed the title
|
@promag Yes I have applied it in both places and tested 5 times and didn't see an issue, great job +1 |
sidhujag
added a commit
to syscoin/syscoin
that referenced
this pull request
Jun 2, 2019
upstream bug fixes related to importwallet
promag
force-pushed
the
2019-06-progressdialog-nullptr
branch
from
1616587 to
d2ae6be
Compare
Contributor
Author
|
@sidhujag updated with your finding. |
|
TACK |
Member
|
utACK d2ae6be |
Member
|
tACK d2ae6be On master (c7cfd20) & src/bitcoin-cli -regtest dumpwallet ~/downloads/wallet
src/bitcoin-cli -regtest importwallet ~/downloads/walletlldb Bitcoin-Qt.app -- -regtest
(lldb) target create "Bitcoin-Qt.app"
Current executable set to 'Bitcoin-Qt.app' (x86_64).
(lldb) settings set -- target.run-args "-regtest"
(lldb) run
Process 10564 launched: '/Users/michael/github/bitcoin/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt' (x86_64)
2019-06-03 09:23:45.314524-0400 Bitcoin-Qt[10564:8394825] MessageTracer: Falling back to default whitelist
Process 10564 stopped
* thread #1, name = 'bitcoin-main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
frame #0: 0x0000000100e516f1 QtWidgets`QWidgetPrivate::close_helper(QWidgetPrivate::CloseMode) + 17
QtWidgets`QWidgetPrivate::close_helper:
-> 0x100e516f1 <+17>: movl 0x140(%rdi), %eax
0x100e516f7 <+23>: movb $0x1, %bl
0x100e516f9 <+25>: testl $0x200, %eax ; imm = 0x200
0x100e516fe <+30>: jne 0x100e5192c ; <+588>
Target 0: (Bitcoin-Qt) stopped.I cannot cause the same crash using this PR. |
Member
|
Yes, it is definitely a bug to hold on to something after |
laanwj
added a commit
that referenced
this pull request
Jun 3, 2019
d2ae6be gui: Set progressDialog to nullptr (João Barbosa) Pull request description: If a progress notification `> 0` arrives immediately after notification `= 100` then `progressDialog` is a dangling pointer. Potential fix for #16134. ACKs for commit d2ae6b: hebasto: utACK d2ae6be fanquake: tACK d2ae6be Tree-SHA512: 300ddde2f27c494b19a5bd4085400d0f5a1d4980fe8cc3c07bfebb037efc35f777215ff1a095eeb16658407e11f04456137393e88a12fdd767b7aac5f12eab5e
Contributor
|
Post-merge utACK d2ae6be FWIW: $ git grep -A1 'deleteLater()'
src/qt/bitcoingui.cpp: progressDialog->deleteLater();
src/qt/bitcoingui.cpp- progressDialog = nullptr;
--
src/qt/paymentserver.cpp: reply->deleteLater();
src/qt/paymentserver.cpp-
--
src/qt/sendcoinsdialog.cpp: ui->entries->takeAt(0)->widget()->deleteLater();
src/qt/sendcoinsdialog.cpp- }
--
src/qt/sendcoinsdialog.cpp: entry->deleteLater();
src/qt/sendcoinsdialog.cpp-
--
src/qt/splashscreen.cpp: deleteLater(); // No more need for this
src/qt/splashscreen.cpp-}
--
src/qt/walletview.cpp: progressDialog->deleteLater();
src/qt/walletview.cpp- progressDialog = nullptr; |
Merged
Member
|
Backported in #16035. |
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Jun 7, 2019
Github-Pull: bitcoin#16135 Rebased-From: d2ae6be
HashUnlimited
pushed a commit
to HashUnlimited/chaincoin
that referenced
this pull request
Aug 23, 2019
Github-Pull: bitcoin#16135 Rebased-From: d2ae6be
Bushstar
pushed a commit
to Bushstar/omnicore
that referenced
this pull request
Aug 24, 2019
Github-Pull: bitcoin#16135 Rebased-From: d2ae6be
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 8, 2020
Summary: The progress dialog pointer must be set to `nullptr` after deletion, else it can cause a crash is `showProgress` is called again with `n>0`. Backport of Core [[bitcoin/bitcoin#16135 | PR16135]] Test Plan: `ninja && ninja check` Run `./bitcoin-qt` Menu `help -> debug -> console ` In the console, run: ``` rescanblockchain dumpwallet /tmp/wallet importwallet /tmp/wallet ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7799
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 1, 2021
d2ae6be gui: Set progressDialog to nullptr (João Barbosa) Pull request description: If a progress notification `> 0` arrives immediately after notification `= 100` then `progressDialog` is a dangling pointer. Potential fix for bitcoin#16134. ACKs for commit d2ae6b: hebasto: utACK d2ae6be fanquake: tACK bitcoin@d2ae6be Tree-SHA512: 300ddde2f27c494b19a5bd4085400d0f5a1d4980fe8cc3c07bfebb037efc35f777215ff1a095eeb16658407e11f04456137393e88a12fdd767b7aac5f12eab5e
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 12, 2021
d2ae6be gui: Set progressDialog to nullptr (João Barbosa) Pull request description: If a progress notification `> 0` arrives immediately after notification `= 100` then `progressDialog` is a dangling pointer. Potential fix for bitcoin#16134. ACKs for commit d2ae6b: hebasto: utACK d2ae6be fanquake: tACK bitcoin@d2ae6be Tree-SHA512: 300ddde2f27c494b19a5bd4085400d0f5a1d4980fe8cc3c07bfebb037efc35f777215ff1a095eeb16658407e11f04456137393e88a12fdd767b7aac5f12eab5e
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
If a progress notification
> 0arrives immediately after notification= 100thenprogressDialogis a dangling pointer.Potential fix for #16134.