Skip to content

wallet: Unify wallet directory lock error message #20094

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
Closed
wants to merge 2 commits into from
Closed

wallet: Unify wallet directory lock error message #20094

wants to merge 2 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Oct 6, 2020

On master (875e1cc) a redundant log message is printed, and it uses "bitcoin" instead of PACKAGE_NAME:

2020-10-06T17:36:44Z Using wallet directory /home/hebasto/walletdir
2020-10-06T17:36:44Z init message: Verifying wallet(s)...
2020-10-06T17:36:44Z Using BerkeleyDB version Berkeley DB 5.3.28: (September  9, 2013)
2020-10-06T17:36:44Z Using wallet /home/hebasto/walletdir/wr-201006/wallet.dat
2020-10-06T17:36:44Z ERROR: Error while attempting to lock directory /home/hebasto/walletdir/wr-201006: Resource temporarily unavailable
2020-10-06T17:36:44Z Cannot obtain a lock on wallet directory /home/hebasto/walletdir/wr-201006. Another instance of bitcoin may be using it.
2020-10-06T17:36:44Z Error: Error initializing wallet database environment "/home/hebasto/walletdir/wr-201006"!
Error: Error initializing wallet database environment "/home/hebasto/walletdir/wr-201006"!
2020-10-06T17:36:44Z Shutdown: In progress...
2020-10-06T17:36:44Z scheduler thread exit
2020-10-06T17:36:44Z Shutdown: done

This PR removes redundant LogPrintf call, and unifies error message wording with the similar datadir lock error message:

2020-10-06T17:34:21Z Using wallet directory /home/hebasto/walletdir
2020-10-06T17:34:21Z init message: Verifying wallet(s)...
2020-10-06T17:34:21Z Using BerkeleyDB version Berkeley DB 5.3.28: (September  9, 2013)
2020-10-06T17:34:21Z Using wallet /home/hebasto/walletdir/wr-201006/wallet.dat
2020-10-06T17:34:21Z ERROR: Error while attempting to lock directory /home/hebasto/walletdir/wr-201006: Resource temporarily unavailable
2020-10-06T17:34:21Z Error: Cannot obtain a lock on wallet directory /home/hebasto/walletdir/wr-201006. Bitcoin Core is probably already running.
Error: Cannot obtain a lock on wallet directory /home/hebasto/walletdir/wr-201006. Bitcoin Core is probably already running.
2020-10-06T17:34:21Z Shutdown: In progress...
2020-10-06T17:34:21Z scheduler thread exit
2020-10-06T17:34:21Z Shutdown: done

@DrahtBot DrahtBot added the Wallet label Oct 6, 2020
@hebasto hebasto force-pushed the 201006-name branch 2 times, most recently from 01df47b to 8598613 Compare October 6, 2020 19:08
@hebasto hebasto marked this pull request as draft October 7, 2020 04:10
@hebasto hebasto marked this pull request as ready for review October 7, 2020 04:15
@maflcko maflcko changed the title wallet, log, qa: Unify wallet directory lock error message wallet: Unify wallet directory lock error message Oct 7, 2020
@maflcko
Copy link
Member

maflcko commented Oct 7, 2020

Concept ACK. Though, this changes a translation string.

@maflcko maflcko added this to the 0.22.0 milestone Oct 7, 2020
maflcko
maflcko reviewed Oct 7, 2020
View reviewed changes
test/functional/wallet_multiwallet.py Outdated
exp_stderr = r"Error: Error initializing wallet database environment \"\S+competing_walletdir\"!"
self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
exp_stderr = "Error: Cannot obtain a lock on wallet directory {0}. {1} is probably already running.".format(competing_wallet_dir, self.config['environment']['PACKAGE_NAME'])
self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr)
Copy link
Member

Choose a reason for hiding this comment

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

Would also be good to temporarily enable the functional tests on appveyor again to see if the full path works on windows as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Done: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/35614117

Copy link
Member

Choose a reason for hiding this comment

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

                                   AssertionError: [node 1] Expected message 
"Error: Cannot obtain a lock on wallet directory C:\Users\appveyor\AppData\Local\Temp\1\test_runner_₿_🏃_20201007_132949\wallet_multiwallet_117\competing_walletdir. Bitcoin Core is probably already running." does not fully match stderr:
                                   
"Error: Cannot obtain a lock on wallet directory C:/Users\appveyor\AppData\Local\Temp\1\test_runner_₿_🏃_20201007_132949\wallet_multiwallet_117\competing_walletdir. Bitcoin Core is probably already running."

Copy link
Contributor

Choose a reason for hiding this comment

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

Lol that's fun ^

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/35676234: wallet_multiwallet.py and feature_filelock.py passed.

@MarcoFalke
Could the "[DO NOT MERGE] ci: Temporarily enable the functional tests on AppVeyor" commit be dropped?

@hebasto hebasto marked this pull request as draft October 7, 2020 18:57
@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 7, 2020 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Oct 7, 2020
Merged
@meshcollider
Copy link
Contributor

Approach ACK

@hebasto hebasto force-pushed the 201006-name branch 2 times, most recently from 5a69040 to 33d49d7 Compare October 10, 2020 13:03
@hebasto hebasto marked this pull request as ready for review October 10, 2020 14:43
@hebasto
Copy link
Member Author

hebasto commented Oct 10, 2020

Fixed Windows paths.

@DrahtBot DrahtBot mentioned this pull request Oct 12, 2020
Merged
@hebasto
Copy link
Member Author

hebasto commented Oct 15, 2020

Rebased 33d49d7 -> 76ab0e7 (pr20094.06 -> pr20094.07) due to the conflict with #19077.

This was referenced Oct 16, 2020
Merged
Merged
@DrahtBot DrahtBot mentioned this pull request Oct 29, 2020
Closed
@hebasto
wallet, log, qa: Unify wallet directory lock error message
729b577 
This change removes redundant LogPrintf call, and unifies error message
wording with the similar datadir lock error message.
@hebasto
[DO NOT MERGE] ci: Temporarily enable the functional tests on AppVeyor
7504cac
@hebasto
Copy link
Member Author

hebasto commented Oct 29, 2020

Rebased 76ab0e7 -> 7504cac (pr20094.07 -> pr20094.08) due to the conflict with #20186.

@DrahtBot DrahtBot mentioned this pull request Oct 30, 2020
Merged
@hebasto hebasto marked this pull request as draft October 30, 2020 10:21
@hebasto
Copy link
Member Author

hebasto commented May 29, 2021

This does not make sense with SQLite wallets.

@hebasto hebasto closed this May 29, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@meshcollider meshcollider meshcollider left review comments

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Wallet
Projects
None yet
Milestone
22.0
Development

Successfully merging this pull request may close these issues.
4 participants
@hebasto @maflcko @DrahtBot @meshcollider