wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree#15583
Merged
laanwj merged 1 commit intobitcoin:masterfrom Mar 14, 2019
Merged
wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree#15583laanwj merged 1 commit intobitcoin:masterfrom
laanwj merged 1 commit intobitcoin:masterfrom
Conversation
Contributor
Author
|
Fixes #15555 (comment). |
Member
|
Concept ACK. |
ryanofsky
approved these changes
Mar 12, 2019
Contributor
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK 8de53d5097c7e0936a15ec738e2580814ee0de9b. Nice fix. I wouldn't have expected incrementing the iterator to throw.
If there's an easy way to add a test, it could be nice to have. But if it requires adding something platform-specific, probably better to skip.
Contributor
Author
I'd like to confirm a gitian build first, cc @MarcoFalke. |
Contributor
Author
|
Thanks @fanquake! |
Member
|
Thanks for catching this @kiv06041992. I was able to reproduce with v0.18.0rc1 binary, as well as when building master from source. This commit fixes it, so tACK 8de53d5097 (test would be nice). I'm even able to create a new wallet and use it, and the wallet will be hidden next time you start (no idea why one would want that). @promag can you copy the steps to reproduce to the description? |
Contributor
Author
Done. |
promag
force-pushed
the
2019-03-fix-listwalletdir
branch
2 times, most recently
from
df3d9d2 to
4c61be6
Compare
Member
|
Although I agree that the current behavior is undesirable, I don't like ignoring errors either. In my experience this makes troubleshooting as good as impossible when something unexpected happens. Please handle the error or at least log it. |
Contributor
Author
|
@laanwj agree, I'll log. |
Contributor
|
EDIT: It looks like failing to increment shouldn't cause infinite loops since |
Contributor
Author
|
Now logging the error and the respective path. Also log error in |
promag
changed the title
ryanofsky
approved these changes
Mar 13, 2019
Contributor
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK d595965c599161b14346a2552867eb2869a097c7. Only change since last review is adding logging. Current PR description ("Log errors in ListWalletDir and IsBerkeleyBtree") seems misleading, though. Main change is not the logging, but making listwalletdir not throw exceptions.
Contributor
|
Gitian builds for commit c3b1cb9 (master):
Gitian builds for commit dab434cdc7d30fef582530766a8d93b93da3ee66 (master and this pull):
|
Member
Thank you, looks good to me now, utACK after squash |
promag
force-pushed
the
2019-03-fix-listwalletdir
branch
from
d595965 to
15c69b1
Compare
promag
changed the title
Contributor
Author
|
Reworded and squashed, thanks. |
laanwj
added a commit
that referenced
this pull request
Mar 14, 2019
…rkeleyBtree 15c69b1 wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree (João Barbosa) Pull request description: Use the `noexcept` members of `boost::filesystem::recursive_directory_iterator` in order to ignore `boost::filesystem::directory_iterator::construct: Permission denied` errors. The errors are logged though. Steps to reproduce the issue: ```sh # 1. create directory for -walletdir without read access: mkdir /tmp/foo && chmod a-r /tmp/foo # 2. run bitcoin-qt and should print an error, but continues running: /Volumes/Bitcoin-Core/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -regtest -walletdir=/tmp/foo /private/tmp/foo: Permission denied # 4. go to File -> Open Wallet and should segfault: EXCEPTION: N5boost10filesystem16filesystem_errorE boost::filesystem::directory_iterator::construct: Permission denied: "/private/tmp/foo" bitcoin in Runaway exception ``` Tree-SHA512: 37e8bf5a1e0defc331030fd511bf9cac2765d01dfbf23e7233f37506e85b8ad07edcde9ba6dae7a2c95700c78d28c7dd248153607381852da96273cb159c4934
Contributor
|
Gitian builds for commit 8e1704c (master):
Gitian builds for commit f5095fb13a91f7a5eccb7230803bb7aeaf686ab9 (master and this pull):
|
HashUnlimited
pushed a commit
to HashUnlimited/chaincoin
that referenced
this pull request
Apr 19, 2019
Github-Pull: bitcoin#15583 Rebased-From: 15c69b1 Tree-SHA512: edef7cafc5a2cb8d3355591a7742ef61454a5dedbb1dc22f6cc6bae42329d887f3f4a054f2aeedf31180051f50b6478d09e204066205699dabc0cb67b0ce4a96
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 4, 2020
…sBerkeleyBtree Summary: wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree (João Barbosa) Pull request description: Use the `noexcept` members of `boost::filesystem::recursive_directory_iterator` in order to ignore `boost::filesystem::directory_iterator::construct: Permission denied` errors. The errors are logged though. Steps to reproduce the issue: ```sh # 1. create directory for -walletdir without read access: mkdir /tmp/foo && chmod a-r /tmp/foo # 2. run bitcoin-qt and should print an error, but continues running: /Volumes/Bitcoin-Core/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -regtest -walletdir=/tmp/foo /private/tmp/foo: Permission denied # 4. go to File -> Open Wallet and should segfault: EXCEPTION: N5boost10filesystem16filesystem_errorE boost::filesystem::directory_iterator::construct: Permission denied: "/private/tmp/foo" bitcoin in Runaway exception ``` --- Backport of Core [[bitcoin/bitcoin#15583 | PR15583]] Test Plan: ninja check check-functional run bitcoin-qt with a read-only -walletdir, do not segfault when hovering over 'create wallet' Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7350
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 11, 2021
…nd IsBerkeleyBtree 15c69b1 wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree (João Barbosa) Pull request description: Use the `noexcept` members of `boost::filesystem::recursive_directory_iterator` in order to ignore `boost::filesystem::directory_iterator::construct: Permission denied` errors. The errors are logged though. Steps to reproduce the issue: ```sh # 1. create directory for -walletdir without read access: mkdir /tmp/foo && chmod a-r /tmp/foo # 2. run bitcoin-qt and should print an error, but continues running: /Volumes/Bitcoin-Core/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -regtest -walletdir=/tmp/foo /private/tmp/foo: Permission denied # 4. go to File -> Open Wallet and should segfault: EXCEPTION: N5boost10filesystem16filesystem_errorE boost::filesystem::directory_iterator::construct: Permission denied: "/private/tmp/foo" bitcoin in Runaway exception ``` Tree-SHA512: 37e8bf5a1e0defc331030fd511bf9cac2765d01dfbf23e7233f37506e85b8ad07edcde9ba6dae7a2c95700c78d28c7dd248153607381852da96273cb159c4934
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 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.
Use the
noexceptmembers ofboost::filesystem::recursive_directory_iteratorin order to ignoreboost::filesystem::directory_iterator::construct: Permission deniederrors. The errors are logged though.Steps to reproduce the issue: