data race: handleVersionMsg #101
Labels
Comments
|
This has been resolved as of commit a55ea10. |
gsalgado
pushed a commit
to gsalgado/btcd
that referenced
this issue
Jul 29, 2014
This commit is the result of several big changes being made to the wallet. In particular, the "handshake" (initial sync to the chain server) was quite racy and required proper synchronization. To make fixing this race easier, several other changes were made to the internal wallet data structures and much of the RPC server ended up being rewritten. First, all account support has been removed. The previous Account struct has been replaced with a Wallet structure, which includes a keystore for saving keys, and a txstore for storing relevant transactions. This decision has been made since it is the opinion of myself and other developers that bitcoind accounts are fundamentally broken (as accounts implemented by bitcoind support both arbitrary address groupings as well as moving balances between accounts -- these are fundamentally incompatible features), and since a BIP0032 keystore is soon planned to be implemented (at which point, "accounts" can return as HD extended keys). With the keystore handling the grouping of related keys, there is no reason have many different Account structs, and the AccountManager has been removed as well. All RPC handlers that take an account option will only work with "" (the default account) or "*" if the RPC allows specifying all accounts. Second, much of the RPC server has been cleaned up. The global variables for the RPC server and chain server client have been moved to part of the rpcServer struct, and the handlers for each RPC method that are looked up change depending on which components have been set. Passthrough requests are also no longer handled specially, but when the chain server is set, a handler to perform the passthrough will be returned if the method is not otherwise a wallet RPC. The notification system for websocket clients has also been rewritten so wallet components can send notifications through channels, rather than requiring direct access to the RPC server itself, or worse still, sending directly to a websocket client's send channel. In the future, this will enable proper registration of notifications, rather than unsolicited broadcasts to every connected websocket client (see issue btcsuite#84). Finally, and the main reason why much of this cleanup was necessary, the races during intial sync with the chain server have been fixed. Previously, when the 'Handshake' was run, a rescan would occur which would perform modifications to Account data structures as notifications were received. Synchronization was provided with a single binary semaphore which serialized all access to wallet and account data. However, the Handshake itself was not able to run with this lock (or else notifications would block), and many data races would occur as both notifications were being handled. If GOMAXPROCS was ever increased beyond 1, btcwallet would always immediately crash due to invalid addresses caused by the data races on startup. To fix this, the single lock for all wallet access has been replaced with mutexes for both the keystore and txstore. Handling of btcd notifications and client requests may now occur simultaneously. GOMAXPROCS has also been set to the number of logical CPUs at the beginning of main, since with the data races fixed, there's no reason to prevent the extra parallelism gained by increasing it. Closes btcsuite#78. Closes btcsuite#101. Closes btcsuite#110.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20:41:15 2014-02-26 [INF] BMGR: New valid peer 88.198.62.174:55909 (inbound) (/getaddr.bitnodes.io:0.1/) 20:41:35 2014-02-26 [INF] BMGR: Lost peer 88.198.62.174:55909 (inbound) ================== WARNING: DATA RACE Write by goroutine 110: main.(*peer).handleVersionMsg() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:278 +0x39a main.(*peer).inHandler() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1112 +0x11a5 Previous read by goroutine 74: main.(*peer).queueHandler() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1260 +0x645 runtime.gosched0() /home/dhill/go/src/pkg/runtime/proc.c:1429 +0xaf github.com/conformal/btcec.(*KoblitzCurve).doubleGeneric() /home/dhill/.go/src/github.com/conformal/btcec/btcec.go:538 +0x260 github.com/conformal/btcec.(*KoblitzCurve).doubleJacobian() /home/dhill/.go/src/github.com/conformal/btcec/btcec.go:575 +0x13f github.com/conformal/btcec.(*KoblitzCurve).ScalarMult() /home/dhill/.go/src/github.com/conformal/btcec/btcec.go:612 +0x20c github.com/conformal/btcec.(*KoblitzCurve).ScalarBaseMult() /home/dhill/.go/src/github.com/conformal/btcec/btcec.go:630 +0xb2 crypto/ecdsa.Verify() /home/dhill/go/src/pkg/crypto/ecdsa/ecdsa.go:147 +0x2f6 github.com/conformal/btcscript.opcodeCheckSig() /home/dhill/.go/src/github.com/conformal/btcscript/opcode.go:1747 +0x920 github.com/conformal/btcscript.(*parsedOpcode).exec() /home/dhill/.go/src/github.com/conformal/btcscript/opcode.go:892 +0x12a github.com/conformal/btcscript.(*Script).Step() /home/dhill/.go/src/github.com/conformal/btcscript/script.go:612 +0x2e9 github.com/conformal/btcscript.(*Script).Execute() /home/dhill/.go/src/github.com/conformal/btcscript/script.go:536 +0x1ea github.com/conformal/btcchain.(*txValidator).validateHandler() /home/dhill/.go/src/github.com/conformal/btcchain/scriptval.go:91 +0x622 Goroutine 110 (running) created at: main.(*peer).Start() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1509 +0x2d2 main.func·023() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1635 +0x8c9 Goroutine 74 (running) created at: main.(*peer).Start() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1513 +0x310 main.func·023() /home/dhill/.go/src/github.com/conformal/btcd/peer.go:1635 +0x8c9 ==================The text was updated successfully, but these errors were encountered: