Catch errors on datadir lock and pidfile delete #6159

pull ajweiss wants to merge 1 commits into bitcoin:master from ajweiss:ajw_1505_datadir_lock_exception changing 1 files +14 −4

ajweiss commented at 2:35 PM on May 19, 2015: contributor

Prevents bad permissions (or other fs related problems) from resulting in hard crashes with cryptic messages on startup and shutdown.

Fixes issues like: #6018 #4059
laanwj added the label Bug on May 20, 2015

Catch errors on datadir lock and pidfile delete

Prevents bad permissions (or other fs related problems) from resulting
in hard crashes with cryptic messages on startup and shutdown.

ffdda4e8a7

in src/init.cpp:None in 00bf1d9945 outdated
```
 872 | +
 873 | +    try {
 874 | +        static boost::interprocess::file_lock lock(pathLockFile.string().c_str());
 875 | +        if (!lock.try_lock())
 876 | +            return InitError(strprintf(_("Cannot obtain a lock on data directory %s. Bitcoin Core is probably already running."), strDataDir));
 877 | +    } catch(const boost::interprocess::interprocess_exception& e) {
```
laanwj commented at 6:25 AM on May 22, 2015:

How can we produce an error like this for testing?

Do we know that the lock will always return a boost::interprocess:interprocess_exception, or should we be more general in catching?

Re: translations, please don't add a translation message _() here for an extremely rare case. Just re-use the above generic message, and log the more detailed error to the debug log.
ajweiss commented at 6:37 AM on May 22, 2015:

a) chmod 000 <datadir>

b) if the boost docs are to be believed, this is the only exception it throws. i could not find a base exception beyond this.

c) i was wary of adding a translation, but this is a pretty ugly error that should be explained clearly. we can't log it (beyond -printtoconsole) because we're blowing up due to an invalid datadir. what do you think?

laanwj commented at 8:49 AM on May 22, 2015:

re a,b) thanks, clear

c) OK. But the problem is that any message returned by .what() will be either in English, or completely incomprehensible.

So may as well keep the entire message in English then. Easier to Google, too.

The alternative would be to have a translated message that explains the probable source of the problem and how to resolve it in understandable language. Adding another translation for "Could not lock data directory" isn't useful.

If you mean to get this in for 0.11, where a string freeze is in progress, I'd go for the first.
ajweiss force-pushed on May 22, 2015
ajweiss commented at 6:22 PM on May 22, 2015: contributor

Ok, pushed. I think I captured the best of both worlds, re-used the existing localized string to indicate there's a problem with locking the datadir. (another copy already running may be misleading in some cases though) Follow it with the english text of the exception to give a hopefully familiar and if not, a google-able clue as to what the problem might be. I fired it up in Spanish and Russian, and it didn't look crazy. Sound good?

in src/init.cpp:None in ffdda4e8a7

 873 | +    try {
 874 | +        static boost::interprocess::file_lock lock(pathLockFile.string().c_str());
 875 | +        if (!lock.try_lock())
 876 | +            return InitError(strprintf(_("Cannot obtain a lock on data directory %s. Bitcoin Core is probably already running."), strDataDir));
 877 | +    } catch(const boost::interprocess::interprocess_exception& e) {
 878 | +        return InitError(strprintf(_("Cannot obtain a lock on data directory %s. Bitcoin Core is probably already running.") + " %s.", strDataDir, e.what()));

laanwj commented at 7:59 AM on May 23, 2015:

Looks like a good solution to me.

laanwj commented at 8:56 AM on May 24, 2015: member

Tested ACK
laanwj merged this on May 24, 2015
laanwj closed this on May 24, 2015
laanwj referenced this in commit e1412d3e96 on May 24, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

ajweiss

laanwj

Labels

Bug

Linked (view graph)

#4059 Incorrect ~/.bitcoin perms raises Boost exception, terminates bitcoin-qt #6018 [ Bitcoin-Qt ] Linux client can't read directories with an @ in its name