• Ferenc Szabo's avatar
    p2p, swarm: fix node up races by granular locking (#18976) · 50b872bf
    Ferenc Szabo authored
    * swarm/network: DRY out repeated giga comment
    
    I not necessarily agree with the way we wait for event propagation.
    But I truly disagree with having duplicated giga comments.
    
    * p2p/simulations: encapsulate Node.Up field so we avoid data races
    
    The Node.Up field was accessed concurrently without "proper" locking.
    There was a lock on Network and that was used sometimes to access
    the  field. Other times the locking was missed and we had
    a data race.
    
    For example: https://github.com/ethereum/go-ethereum/pull/18464
    The case above was solved, but there were still intermittent/hard to
    reproduce races. So let's solve the issue permanently.
    
    resolves: ethersphere/go-ethereum#1146
    
    * p2p/simulations: fix unmarshal of simulations.Node
    
    Making Node.Up field private in 13292ee897e345045fbfab3bda23a77589a271c1
    broke TestHTTPNetwork and TestHTTPSnapshot. Because the default
    UnmarshalJSON does not handle unexported fields.
    
    Important: The fix is partial and not proper to my taste. But I cut
    scope as I think the fix may require a change to the current
    serialization format. New ticket:
    https://github.com/ethersphere/go-ethereum/issues/1177
    
    * p2p/simulations: Add a sanity test case for Node.Config UnmarshalJSON
    
    * p2p/simulations: revert back to defer Unlock() pattern for Network
    
    It's a good patten to call `defer Unlock()` right after `Lock()` so
    (new) error cases won't miss to unlock. Let's get back to that pattern.
    
    The patten was abandoned in 85a79b3a,
    while fixing a data race. That data race does not exist anymore,
    since the Node.Up field got hidden behind its own lock.
    
    * p2p/simulations: consistent naming for test providers Node.UnmarshalJSON
    
    * p2p/simulations: remove JSON annotation from private fields of Node
    
    As unexported fields are not serialized.
    
    * p2p/simulations: fix deadlock in Network.GetRandomDownNode()
    
    Problem: GetRandomDownNode() locks -> getDownNodeIDs() ->
    GetNodes() tries to lock -> deadlock
    
    On Network type, unexported functions must assume that `net.lock`
    is already acquired and should not call exported functions which
    might try to lock again.
    
    * p2p/simulations: ensure method conformity for Network
    
    Connect* methods were moved to p2p/simulations.Network from
    swarm/network/simulation. However these new methods did not follow
    the pattern of Network methods, i.e., all exported method locks
    the whole Network either for read or write.
    
    * p2p/simulations: fix deadlock during network shutdown
    
    `TestDiscoveryPersistenceSimulationSimAdapter` often got into deadlock.
    The execution was stuck on two locks, i.e, `Kademlia.lock` and
    `p2p/simulations.Network.lock`. Usually the test got stuck once in each
    20 executions with high confidence.
    
    `Kademlia` was stuck in `Kademlia.EachAddr()` and `Network` in
    `Network.Stop()`.
    
    Solution: in `Network.Stop()` `net.lock` must be released before
    calling `node.Stop()` as stopping a node (somehow - I did not find
    the exact code path) causes `Network.InitConn()` to be called from
    `Kademlia.SuggestPeer()` and that blocks on `net.lock`.
    
    Related ticket: https://github.com/ethersphere/go-ethereum/issues/1223
    
    * swarm/state: simplify if statement in DBStore.Put()
    
    * p2p/simulations: remove faulty godoc from private function
    
    The comment started with the wrong method name.
    
    The method is simple and self explanatory. Also, it's private.
    => Let's just remove the comment.
    50b872bf
Name
Last commit
Last update
.github Loading commit data...
accounts Loading commit data...
build Loading commit data...
cmd Loading commit data...
common Loading commit data...
consensus Loading commit data...
console Loading commit data...
containers/docker Loading commit data...
contracts Loading commit data...
core Loading commit data...
crypto Loading commit data...
dashboard Loading commit data...
docs/audits Loading commit data...
eth Loading commit data...
ethclient Loading commit data...
ethdb Loading commit data...
ethstats Loading commit data...
event Loading commit data...
graphql Loading commit data...
internal Loading commit data...
les Loading commit data...
light Loading commit data...
log Loading commit data...
metrics Loading commit data...
miner Loading commit data...
mobile Loading commit data...
node Loading commit data...
p2p Loading commit data...
params Loading commit data...
rlp Loading commit data...
rpc Loading commit data...
signer Loading commit data...
swarm Loading commit data...
tests Loading commit data...
trie Loading commit data...
vendor Loading commit data...
whisper Loading commit data...
.dockerignore Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gitmodules Loading commit data...
.mailmap Loading commit data...
.travis.yml Loading commit data...
AUTHORS Loading commit data...
COPYING Loading commit data...
COPYING.LESSER Loading commit data...
Dockerfile Loading commit data...
Dockerfile.alltools Loading commit data...
Makefile Loading commit data...
README.md Loading commit data...
appveyor.yml Loading commit data...
circle.yml Loading commit data...
interfaces.go Loading commit data...