Commit Graph

640 Commits

Author SHA1 Message Date
Wladimir J. van der Laan
aab1e55860
Merge #9987: Remove unused code
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
2017-03-18 10:10:33 +01:00
practicalswift
8dc957ae06 Remove unused code 2017-03-17 04:37:43 +01:00
Wladimir J. van der Laan
8b789d8141
Merge #9921: build: Probe MSG_DONTWAIT in the same way as MSG_NOSIGNAL
a4d1c9f compat: use `unsigned int` instead of `u_int` (Wladimir J. van der Laan)
25da1ee build: cleanup: define MSG_DONTWAIT/MSG_NO_SIGNAL locally (Wladimir J. van der Laan)
c459d50 build: Probe MSG_DONTWAIT in the same way as MSG_NOSIGNAL (Wladimir J. van der Laan)

Tree-SHA512: 60d79d69439bb181465e4244aa5ddc28bbd84f69c0ca0c753956b3798c9022394e29d791bc085fe7ffb1268c64c789a57e24797daad63525bb776088188ff9ae
2017-03-16 12:03:00 +01:00
Matt Corallo
819b513a54 Add missing braces in semaphore posts in net 2017-03-08 14:55:28 -05:00
Matt Corallo
e007b243c4 Fix shutdown hang with >= 8 -addnodes set
We previously would block waiting for a CSemaphoreGrant in
ThreadOpenAddedConnections, when we did not need to. This would
block as the posts in CConnman shutdown were both to the wrong
semaphore and in the wrong location.
2017-03-08 14:41:57 -05:00
Matt Corallo
73296f54d6 CScheduler boost->std::function, use millisecs for times, not secs 2017-03-06 18:33:50 -05:00
Wladimir J. van der Laan
25da1ee36c build: cleanup: define MSG_DONTWAIT/MSG_NO_SIGNAL locally
Define MSG_DONTWAIT and MSG_NO_SIGNAL in the implementation files that
use them (`net.cpp` and `netbase.cpp`), instead of compat.h which is
included all over the place.

This avoids putting them in the global namespace, as defining them as 0
is a hack that works for our specific usage, but it is not a general
solution.

Also makes sure they are defined only once so the `!defined(MSG_x)` guard can go.
2017-03-05 09:47:29 +00:00
keystrike
6d37ee8314 Trivial: Debug log ambiguity fix for peer addrs
This line can be misinterpreted as loading wallet addresses which is confusing, especially when the wallet is disabled.
2017-02-28 22:14:24 -03:00
Wladimir J. van der Laan
b08656e343
Merge #9715: Disconnect peers which we do not receive VERACKs from within 60 sec
66f861a Add a test for P2P inactivity timeouts (Matt Corallo)
b436f92 qa: Expose on-connection to mininode listeners (Matt Corallo)
8aaba7a qa: mininode learns when a socket connects, not its first action (Matt Corallo)
2cbd119 Disconnect peers which we do not receive VERACKs from within 60 sec (Matt Corallo)
2017-02-14 14:35:15 +01:00
Matt Corallo
db2dc7a58c Move CNode::addrLocal access behind locked accessors 2017-02-10 11:32:41 -05:00
Matt Corallo
036073bf87 Move CNode::addrName accesses behind locked accessors 2017-02-10 11:32:41 -05:00
Matt Corallo
22b4966a29 Move [clean|str]SubVer writes/copyStats into a lock 2017-02-10 11:32:41 -05:00
Matt Corallo
512731bed0 Access fRelayTxes with cs_filter lock in copyStats 2017-02-10 11:32:40 -05:00
Matt Corallo
ae683c1b19 Avoid copying CNodeStats to make helgrind OK with buggy std::string 2017-02-10 11:32:40 -05:00
Matt Corallo
644f1234e2 Make nTimeConnected const in CNode 2017-02-10 11:32:40 -05:00
Cory Fields
321d0fc6b6 net: fix a few races. Credit @TheBlueMatt
These are (afaik) all long-standing races or concurrent accesses. Going
forward, we can clean these up so that they're not all individual atomic
accesses.

- Reintroduce cs_vRecv to guard receive-specific vars
- Lock vRecv/vSend for CNodeStats
- Make some vars atomic.
- Only set the connection time in CNode's constructor so that it doesn't change
2017-02-10 11:32:39 -05:00
Wladimir J. van der Laan
2447c1024e
Merge #9698: net: fix socket close race
9a0b784 net: add a lock around hSocket (Cory Fields)
45e2e08 net: rearrange so that socket accesses can be grouped together (Cory Fields)
2017-02-10 12:42:53 +01:00
Wladimir J. van der Laan
dd163f5788
Merge #9674: Always enforce strict lock ordering (try or not)
618ee92 Further-enforce lockordering by enforcing directly after TRY_LOCKs (Matt Corallo)
2a962d4 Fixup style a bit by moving { to the same line as if statements (Matt Corallo)
8465631 Always enforce lock strict lock ordering (try or not) (Matt Corallo)
fd13eca Lock cs_vSend and cs_inventory in a consistent order even in TRY (Matt Corallo)
2017-02-08 14:46:43 +01:00
Matt Corallo
2cbd1196b7 Disconnect peers which we do not receive VERACKs from within 60 sec 2017-02-07 17:44:46 -05:00
Cory Fields
9a0b784dea net: add a lock around hSocket 2017-02-06 14:48:50 -05:00
Cory Fields
45e2e08561 net: rearrange so that socket accesses can be grouped together 2017-02-06 14:48:50 -05:00
Wladimir J. van der Laan
09e0c28f85
Merge #9659: Net: Turn some methods and params/variables const
0729102 Net: pass interruptMsgProc as const where possible (Jorge Timón)
fc7f2ff Net: Make CNetMsgMaker more const (Jorge Timón)
d45955f Net: CConnman: Make some methods const (Jorge Timón)
2017-02-06 14:34:53 +01:00
Matt Corallo
2a962d4540 Fixup style a bit by moving { to the same line as if statements 2017-02-04 16:44:05 -05:00
Wladimir J. van der Laan
7821db30e1
Merge #9671: Fix super-unlikely race introduced in 236618061a
885cfdd Fix super-unlikely race introduced in 236618061a (Matt Corallo)
2017-02-04 11:39:17 +01:00
Matt Corallo
fd13eca147 Lock cs_vSend and cs_inventory in a consistent order even in TRY 2017-02-02 20:03:46 -05:00
Cory Fields
08bb6f4ed4 net: log an error rather than asserting if send version is misused
Also cleaned up the comments and moved from the header to the .cpp so that
logging headers aren't needed from net.h
2017-02-02 16:14:16 -05:00
Cory Fields
12752af0cc net: don't run callbacks on nodes that haven't completed the version handshake
Since ForEach* are can be used to send messages to  all nodes, the caller may
end up sending a message before the version handshake is complete. To limit
this, filter out these nodes. While we're at it, may as well filter out
disconnected nodes as well.

Delete unused methods rather than updating them.
2017-02-02 16:14:16 -05:00
Matt Corallo
885cfdd217 Fix super-unlikely race introduced in 236618061a
Once the CNode has been added to vNodes, it is possible that it is
disconnected+deleted in the socket handler thread. However, after
that we now call InitializeNode, which accesses the pnode.

helgrind managed to tickle this case (somehow), but I suspect it
requires in immensely braindead scheduler.
2017-02-02 13:51:57 -05:00
Jorge Timón
d45955fa09
Net: CConnman: Make some methods const 2017-01-31 23:20:26 +01:00
Wladimir J. van der Laan
36966a1c0e
Merge #9626: Clean up a few CConnman cs_vNodes/CNode things
2366180 Do not add to vNodes until fOneShot/fFeeler/fAddNode have been set (Matt Corallo)
3c37dc4 Ensure cs_vNodes is held when using the return value from FindNode (Matt Corallo)
5be0190 Delete some unused (and broken) functions in CConnman (Matt Corallo)
2017-01-30 12:48:43 +01:00
Wladimir J. van der Laan
3f9f9629cc
Merge #9606: net: Consistently use GetTimeMicros() for inactivity checks
99464bc net: Consistently use GetTimeMicros() for inactivity checks (Suhas Daftuar)
2017-01-26 09:57:45 +01:00
Matt Corallo
236618061a Do not add to vNodes until fOneShot/fFeeler/fAddNode have been set 2017-01-25 18:59:16 -05:00
Suhas Daftuar
99464bc38e net: Consistently use GetTimeMicros() for inactivity checks
The use of mocktime in test logic means that comparisons between
GetTime() and GetTimeMicros()/1000000 are unreliable since the former
can use mocktime values while the latter always gets the system clock;
this changes the networking code's inactivity checks to consistently
use the system clock for inactivity comparisons.

Also remove some hacks from setmocktime() that are no longer needed,
now that we're using the system clock for nLastSend and nLastRecv.
2017-01-25 09:48:14 -05:00
Matt Corallo
3c37dc40d3 Ensure cs_vNodes is held when using the return value from FindNode 2017-01-24 17:01:45 -05:00
Matt Corallo
5be01906e5 Delete some unused (and broken) functions in CConnman 2017-01-24 17:01:43 -05:00
Matt Corallo
376b3c2c6e Make the cs_sendProcessing a LOCK instead of a TRY_LOCK
Technically cs_sendProcessing is entirely useless now because it
is only ever taken on the one MessageHandler thread, but because
there may be multiple of those in the future, it is left in place
2017-01-13 10:34:38 -08:00
Matt Corallo
d7c58ad514 Split CNode::cs_vSend: message processing and message sending
cs_vSend is used for two purposes - to lock the datastructures used
to queue messages to place on the wire and to only call
SendMessages once at a time per-node. I believe SendMessages used
to access some of the vSendMsg stuff, but it doesn't anymore, so
these locks do not need to be on the same mutex, and also make
deadlocking much more likely.
2017-01-13 10:34:37 -08:00
Pieter Wuille
8b66bf74e2
Merge #9441: Net: Massive speedup. Net locks overhaul
e60360e net: remove cs_vRecvMsg (Cory Fields)
991955e net: add a flag to indicate when a node's send buffer is full (Cory Fields)
c6e8a9b net: add a flag to indicate when a node's process queue is full (Cory Fields)
4d712e3 net: add a new message queue for the message processor (Cory Fields)
c5a8b1b net: rework the way that the messagehandler sleeps (Cory Fields)
c72cc88 net: remove useless comments (Cory Fields)
ef7b5ec net: Add a simple function for waking the message handler (Cory Fields)
f5c36d1 net: record bytes written before notifying the message processor (Cory Fields)
60befa3 net: handle message accounting in ReceiveMsgBytes (Cory Fields)
56212e2 net: set message deserialization version when it's actually time to deserialize (Cory Fields)
0e973d9 net: remove redundant max sendbuffer size check (Cory Fields)
6042587 net: wait until the node is destroyed to delete its recv buffer (Cory Fields)
f6315e0 net: only disconnect if fDisconnect has been set (Cory Fields)
5b4a8ac net: make GetReceiveFloodSize public (Cory Fields)
e5bcd9c net: make vRecvMsg a list so that we can use splice() (Cory Fields)
53ad9a1 net: fix typo causing the wrong receive buffer size (Cory Fields)
2017-01-13 10:02:18 -08:00
Cory Fields
e60360e139 net: remove cs_vRecvMsg
vRecvMsg is now only touched by the socket handler thread.

The accounting vars (nRecvBytes/nLastRecv/mapRecvBytesPerMsgCmd) are also
only used by the socket handler thread, with the exception of queries from
rpc/gui. These accesses are not threadsafe, but they never were. This needs to
be addressed separately.

Also, update comment describing data flow
2017-01-12 23:08:08 -05:00
Cory Fields
991955ee81 net: add a flag to indicate when a node's send buffer is full
Similar to the recv flag, but this one indicates whether or not the net's send
buffer is full.

The socket handler checks the send queue when a new message is added and pauses
if necessary, and possibly unpauses after each message is drained from its buffer.
2017-01-12 23:05:59 -05:00
Cory Fields
c6e8a9bcff net: add a flag to indicate when a node's process queue is full
Messages are dumped very quickly from the socket handler to the processor, so
it's the depth of the processing queue that's interesting.

The socket handler checks the process queue's size during the brief message
hand-off and pauses if necessary, and the processor possibly unpauses each time
a message is popped off of its queue.
2017-01-12 23:05:47 -05:00
Cory Fields
4d712e366c net: add a new message queue for the message processor
This separates the storage of messages from the net and queued messages for
processing, allowing the locks to be split.
2017-01-12 23:05:25 -05:00
Cory Fields
c5a8b1b946 net: rework the way that the messagehandler sleeps
In order to sleep accurately, the message handler needs to know if _any_ node
has more processing that it should do before the entire thread sleeps.

Rather than returning a value that represents whether ProcessMessages
encountered a message that should trigger a disconnnect, interpret the return
value as whether or not that node has more work to do.

Also, use a global fProcessWake value that can be set by other threads,
which takes precedence (for one cycle) over the messagehandler's decision.

Note that the previous behavior was to only process one message per loop
(except in the case of a bad checksum or invalid header). That was changed in
PR #3180.

The only change here in that regard is that the current node now falls to the
back of the processing queue for the bad checksum/invalid header cases.
2017-01-12 23:05:24 -05:00
Cory Fields
ef7b5ecbb7 net: Add a simple function for waking the message handler
This may be used publicly in the future
2017-01-12 23:05:09 -05:00
Cory Fields
f5c36d19b6 net: record bytes written before notifying the message processor 2017-01-12 23:05:09 -05:00
Cory Fields
60befa3997 net: handle message accounting in ReceiveMsgBytes
This allows locking to be pushed down to only where it's needed

Also reuse the current time rather than checking multiple times.
2017-01-12 23:05:09 -05:00
Douglas Roark
cc0589639c
Remove stray semicolon (Fix empty body warning)
Empty body introduced by commit #9319 should not be empty.
2017-01-07 09:49:14 -08:00
Gregory Maxwell
032ba3f066 RPC help documentation for addnode peerinfo.
Also adds a comment about the netgroup exclusion behavior.
2017-01-05 19:02:15 +00:00
Gregory Maxwell
50bd12ce0c Break addnode out from the outbound connection limits.
Previously addnodes were in competition with outbound connections
 for access to the eight outbound slots.

One result of this is that frequently a node with several addnode
 configured peers would end up connected to none of them, because
 while the addnode loop was in its two minute sleep the automatic
 connection logic would fill any free slots with random peers.
 This is particularly unwelcome to users trying to maintain links
 to specific nodes for fast block relay or purposes.

Another result is that a group of nine or more nodes which are
 have addnode configured towards each other can become partitioned
 from the public network.

This commit introduces a new limit of eight connections just for
 addnode peers which is not subject to any of the other connection
 limitations (including maxconnections).

The choice of eight is sufficient so that under no condition would
 a user find themselves connected to fewer addnoded peers than
 previously.  It is also low enough that users who are confused
 about the significance of more connections and have gotten too
 copy-and-paste happy will not consume more than twice the slot
 usage of a typical user.

Any additional load on the network resulting from this will likely
 be offset by a reduction in users applying even more wasteful
 workaround for the prior behavior.

The retry delays are reduced to avoid nodes sitting around without
 their added peers up, but are still sufficient to prevent overly
 aggressive repeated connections.  The reduced delays also make
 the system much more responsive to the addnode RPC.

Ban-disconnects are also exempted for peers added via addnode since
 the outbound addnode logic ignores bans.  Previously it would ban
 an addnode then immediately reconnect to it.

A minor change was also made to CSemaphoreGrant so that it is
 possible to re-acquire via an object whos grant was moved.
2017-01-05 19:02:09 +00:00
Cory Fields
56212e20ac net: set message deserialization version when it's actually time to deserialize
We'll soon no longer have access to vRecvMsg, and this is more intuitive anyway.
2017-01-04 10:10:35 -05:00