* [Bitcoin-development] 0.4rc1 known bugs @ 2011-09-04 0:13 Gavin Andresen 2011-09-04 2:43 ` Matt Corallo ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Gavin Andresen @ 2011-09-04 0:13 UTC (permalink / raw) To: Bitcoin Dev Quick status update on 0.4; I probably won't have time to tackle these properly before Tuesday: + sipa found what looks like a deadlock between the addr-handling and IRC-join-handling code. + UukGoblin reports a deadlock problem on a bitcoind handling getwork requests. If you want to get more familiar with the bitcoin code and you have a lot of patience, tracking down deadlocks a great way to do it. + ArtForz found a performance bug with transactions that have thousands of inputs and outputs on the solidcoin test network. (not as big an issue for bitcoin due to fees being based on transaction size, but still worrying) -- -- Gavin Andresen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-04 0:13 [Bitcoin-development] 0.4rc1 known bugs Gavin Andresen @ 2011-09-04 2:43 ` Matt Corallo 2011-09-05 7:25 ` Michael Grønager ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Matt Corallo @ 2011-09-04 2:43 UTC (permalink / raw) To: bitcoin-development On Sat, 2011-09-03 at 20:13 -0400, Gavin Andresen wrote: > Quick status update on 0.4; I probably won't have time to tackle these > properly before Tuesday: > > + sipa found what looks like a deadlock between the addr-handling and > IRC-join-handling code. > + UukGoblin reports a deadlock problem on a bitcoind handling getwork requests. > > If you want to get more familiar with the bitcoin code and you have a > lot of patience, tracking down deadlocks a great way to do it. > > + ArtForz found a performance bug with transactions that have > thousands of inputs and outputs on the solidcoin test network. > (not as big an issue for bitcoin due to fees being based on > transaction size, but still worrying) > + (my fault) Gitian doesnt build properly. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-04 0:13 [Bitcoin-development] 0.4rc1 known bugs Gavin Andresen 2011-09-04 2:43 ` Matt Corallo @ 2011-09-05 7:25 ` Michael Grønager 2011-09-05 12:42 ` Luke-Jr [not found] ` <20110904115926.GA16476@ulyssis.org> 2011-09-06 20:55 ` Luke-Jr 3 siblings, 1 reply; 9+ messages in thread From: Michael Grønager @ 2011-09-05 7:25 UTC (permalink / raw) To: Gavin Andresen; +Cc: Bitcoin Dev Hi Gavin, Did a quick compile and run (bitcoind, Ubuntu 10.04.3 LTS) Findings - compile (I do not use the UPNP feature): in the makefile.unix I have to change the: USE_UPNP:=0 to USE_UPNP:= i.e. it is defined if it is "0" ! running: no apparent issues (I have never managed to trigger the deadlocks.?.) Nice job, but a quick cleanup of interfaces and classes (one file pr class, all interfaces defined in headers) would really be nice... Would be happy to do it myself, as it would greatly enhance the flexibility of the code and be a first step towards a more library/interface like split. Cheers, Michael On 04/09/2011, at 02:13, Gavin Andresen wrote: > Quick status update on 0.4; I probably won't have time to tackle these > properly before Tuesday: > > + sipa found what looks like a deadlock between the addr-handling and > IRC-join-handling code. > + UukGoblin reports a deadlock problem on a bitcoind handling getwork requests. > > If you want to get more familiar with the bitcoin code and you have a > lot of patience, tracking down deadlocks a great way to do it. > > + ArtForz found a performance bug with transactions that have > thousands of inputs and outputs on the solidcoin test network. > (not as big an issue for bitcoin due to fees being based on > transaction size, but still worrying) > > -- > -- > Gavin Andresen > > ------------------------------------------------------------------------------ > Special Offer -- Download ArcSight Logger for FREE! > Finally, a world-class log management solution at an even better > price-free! And you'll get a free "Love Thy Logs" t-shirt when you > download Logger. Secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsisghtdev2dev > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development Michael Gronager, PhD Owner Ceptacle / NDGF Director, NORDUnet A/S Jens Juels Gade 33 2100 Copenhagen E Mobile: +45 31 62 14 01 E-mail: gronager@ceptacle.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-05 7:25 ` Michael Grønager @ 2011-09-05 12:42 ` Luke-Jr 2011-09-05 12:47 ` Michael Grønager 0 siblings, 1 reply; 9+ messages in thread From: Luke-Jr @ 2011-09-05 12:42 UTC (permalink / raw) To: bitcoin-development On Monday, September 05, 2011 3:25:47 AM Michael Grønager wrote: > Findings - compile (I do not use the UPNP feature): > in the makefile.unix I have to change the: > USE_UPNP:=0 > to > USE_UPNP:= > i.e. it is defined if it is "0" ! Yes, the default is "UPnP supported, disabled by default" (USE_UPNP=0), not "UPnP not supported" (USE_UPNP=). This is documented in build-unix.txt ... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-05 12:42 ` Luke-Jr @ 2011-09-05 12:47 ` Michael Grønager 0 siblings, 0 replies; 9+ messages in thread From: Michael Grønager @ 2011-09-05 12:47 UTC (permalink / raw) To: Luke-Jr; +Cc: bitcoin-development Sorry, by bad - first clean checkout for quite a while (must have changed it earlier myself...). /M On 05/09/2011, at 14:42, Luke-Jr wrote: > On Monday, September 05, 2011 3:25:47 AM Michael Grønager wrote: >> Findings - compile (I do not use the UPNP feature): >> in the makefile.unix I have to change the: >> USE_UPNP:=0 >> to >> USE_UPNP:= >> i.e. it is defined if it is "0" ! > > Yes, the default is "UPnP supported, disabled by default" (USE_UPNP=0), not > "UPnP not supported" (USE_UPNP=). This is documented in build-unix.txt ... > > ------------------------------------------------------------------------------ > Special Offer -- Download ArcSight Logger for FREE! > Finally, a world-class log management solution at an even better > price-free! And you'll get a free "Love Thy Logs" t-shirt when you > download Logger. Secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsisghtdev2dev > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development Michael Gronager, PhD Owner Ceptacle / NDGF Director, NORDUnet A/S Jens Juels Gade 33 2100 Copenhagen E Mobile: +45 31 62 14 01 E-mail: gronager@ceptacle.com ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20110904115926.GA16476@ulyssis.org>]
* Re: [Bitcoin-development] 0.4rc1 known bugs [not found] ` <20110904115926.GA16476@ulyssis.org> @ 2011-09-06 11:55 ` Pieter Wuille 2011-09-06 17:59 ` Gavin Andresen 0 siblings, 1 reply; 9+ messages in thread From: Pieter Wuille @ 2011-09-06 11:55 UTC (permalink / raw) To: Gavin Andresen; +Cc: Bitcoin Dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-06 11:55 ` Pieter Wuille @ 2011-09-06 17:59 ` Gavin Andresen 0 siblings, 0 replies; 9+ messages in thread From: Gavin Andresen @ 2011-09-06 17:59 UTC (permalink / raw) To: Bitcoin Dev Nice work, Detective Wuille! Patch for the deadlock issue: https://github.com/bitcoin/bitcoin/pull/500 I took a different approach to fix from the one Pieter suggested, performing the database operation after the cs_mapaddresses deadlock is released. Please review to check my logic, it did survive my start/stop/restart... stress test. And I did review every place in the code that starts a database transaction, to look for similar issues, and they are all OK. RE: improving DEBUG_LOCKORDER: requires some thought. Deadlocks are still possible with TRY_CRITICAL_SECTION, if some codepaths TRY and some don't. On Tue, Sep 6, 2011 at 7:55 AM, Pieter Wuille <pieter.wuille@cs.kuleuven.be> wrote: > 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. -- -- Gavin Andresen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-04 0:13 [Bitcoin-development] 0.4rc1 known bugs Gavin Andresen ` (2 preceding siblings ...) [not found] ` <20110904115926.GA16476@ulyssis.org> @ 2011-09-06 20:55 ` Luke-Jr 2011-09-07 15:07 ` Gavin Andresen 3 siblings, 1 reply; 9+ messages in thread From: Luke-Jr @ 2011-09-06 20:55 UTC (permalink / raw) To: bitcoin-development Got a fix for the encrypted-wallet mining issue: - unique_coinbase It depends on (and merges) the getwork_dedupe fix already common on pools and other miners who pay attention to the latest mining fixes. To merge: git fetch git://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin.git \ unique_coinbase && git merge FETCH_HEAD ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bitcoin-development] 0.4rc1 known bugs 2011-09-06 20:55 ` Luke-Jr @ 2011-09-07 15:07 ` Gavin Andresen 0 siblings, 0 replies; 9+ messages in thread From: Gavin Andresen @ 2011-09-07 15:07 UTC (permalink / raw) To: Luke-Jr; +Cc: bitcoin-development On Tue, Sep 6, 2011 at 4:55 PM, Luke-Jr <luke@dashjr.org> wrote: > Got a fix for the encrypted-wallet mining issue: > - unique_coinbase Turned this into a pull request: https://github.com/bitcoin/bitcoin/pull/505 I reviewed the code but have not tested. Rough sketch of a test plan: Run clean testnet-in-a-box bitcoind, with -keypool=1 Encrypt the wallet Run bitcoind getnewaddress until it tell you keypool is exhausted Generate a couple of blocks via internal miner -- verify: coinbase transactions have unique txids even though they pay-to default key Generate a couple of blocks via getwork RPC call -- verify: coinbase transactions have unique txids -- -- Gavin Andresen ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-09-07 15:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-04 0:13 [Bitcoin-development] 0.4rc1 known bugs 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 2011-09-06 17:59 ` Gavin Andresen 2011-09-06 20:55 ` Luke-Jr 2011-09-07 15:07 ` Gavin Andresen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox