From: Pieter Wuille <pieter.wuille@cs•kuleuven.be>
To: Gavin Andresen <gavinandresen@gmail•com>
Cc: Bitcoin Dev <bitcoin-development@lists•sourceforge.net>
Subject: Re: [Bitcoin-development] 0.4rc1 known bugs
Date: Tue, 6 Sep 2011 13:55:30 +0200 [thread overview]
Message-ID: <CAPg+sBjy1FANzv5N7P4kx0Djqz-P2XbqQAFTxnK-MVu8erja+g@mail.gmail.com> (raw)
In-Reply-To: <20110904115926.GA16476@ulyssis.org>
On Sun, Sep 4, 2011 at 13:59, Pieter Wuille
<pieter.wuille@cs•kuleuven.be> wrote:
> I've compiled bitcoind with Gavin's DEBUG_LOCKORDER, and fixed two potential
> reported deadlock issues (see https://github.com/sipa/bitcoin/commits/lockfixes).
My mistake: these are not actual potential deadlocks, as all locking
of cs_vRecv/cs_vSend
happens inside TRY_CRITICAL_SECTION blocks. Gavin, maybe you can add the rule to
your debug code that ignores critical sections which are only locked
through TRY_...?
>> + sipa found what looks like a deadlock between the addr-handling and
>> IRC-join-handling code.
Regarding the actual deadlock between IRC seeding and AddAddress:
Internally, DB also uses pthreads to implement the txn_begin()/commit() scheme,
though I'm not sure with which granularity. These need to be taken into account
when searching for deadlocks, but are obviously not detected by
DEBUG_LOCKORDER.
In particular here, the processing of "addr" created a db transaction for the
entire message, while only locking cs_mapAddresses inside AddAddress. For
IRC seeded addresses however, no db tx was precreated, and AddAddress first
locked cs_mapAddress, and then did the database write (causing a lock) inside.
A solution: in main.cpp, ProcessMessage, case "addr":
// Store the new addresses
CAddrDB addrDB;
+ CRITICAL_BLOCK(cs_mapAddresses) {
addrDB.TxnBegin();
int64 nNow = GetAdjustedTime();
int64 nSince = nNow - 10 * 60;
}
}
addrDB.TxnCommit(); // Save addresses (it's ok if this fails)
+ }
if (vAddr.size() < 1000)
which makes sure that cs_mapAddresses is always entered before starting
a database transaction.
However, there may be similar issues in other place where TxnBegin is called
explicitly. Also, maybe there are other solutions, like changing BDB parameters
that make the db transaction fail instead of block, for example.
--
Pieter
next prev parent reply other threads:[~2011-09-06 11:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-04 0:13 Gavin Andresen
2011-09-04 2:43 ` Matt Corallo
2011-09-05 7:25 ` Michael Grønager
2011-09-05 12:42 ` Luke-Jr
2011-09-05 12:47 ` Michael Grønager
[not found] ` <20110904115926.GA16476@ulyssis.org>
2011-09-06 11:55 ` Pieter Wuille [this message]
2011-09-06 17:59 ` Gavin Andresen
2011-09-06 20:55 ` Luke-Jr
2011-09-07 15:07 ` Gavin Andresen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPg+sBjy1FANzv5N7P4kx0Djqz-P2XbqQAFTxnK-MVu8erja+g@mail.gmail.com \
--to=pieter.wuille@cs$(echo .)kuleuven.be \
--cc=bitcoin-development@lists$(echo .)sourceforge.net \
--cc=gavinandresen@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox