Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bitcoin-qt -noconnect uses 100% CPU #1664

Closed
dooglus opened this issue Aug 9, 2012 · 11 comments · Fixed by #1694
Closed

bitcoin-qt -noconnect uses 100% CPU #1664

dooglus opened this issue Aug 9, 2012 · 11 comments · Fixed by #1694

Comments

@dooglus
Copy link
Contributor

dooglus commented Aug 9, 2012

Maybe I'm doing it wrong, but if I specify "-noconnect" in an attempt to have bitcoin-qt not connect to any peers, the ThreadOpenConnections2() function goes into a tight loop that runs forever.

The following 1-line change fixes the problem:

diff --git a/src/net.cpp b/src/net.cpp
index e10829a..fe1712a 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1367,7 +1367,7 @@ void ThreadOpenConnections2(void* parg)
     printf("ThreadOpenConnections started\n");

     // Connect to specific addresses
-    if (mapArgs.count("-connect"))
+    if (mapArgs.count("-connect") && mapMultiArgs["-connect"].size())
     {
         for (int64 nLoop = 0;; nLoop++)
         {
@luke-jr
Copy link
Member

luke-jr commented Aug 9, 2012

I don't think -noconnect was ever supported as a valid option.

@gmaxwell
Copy link
Contributor

gmaxwell commented Aug 9, 2012

As Luke says, that wasn't an intended option as far as I know. But it sounds like a reasonable thing to support, though we should probably also audit the other cases were a no wasn't really anticipated.

@laanwj
Copy link
Member

laanwj commented Aug 18, 2012

This also appears to happen when it simply has no nodes to connect to, even without -noconnect. In my case I have a testnet node that has no addresses to connect to, so it keeps cycling through them and the "bitcoin-opencon" thread takes a lot of CPU time.

In my case, it keeps selecting the same peer every half a second (with a nonstandard port #, so after 50 tries), trying to connect to it:

trying connection XX.XX.XX.XX:18334 lastseen=1542.8hrs
connect() failed after select(): Connection refused

Not yet sure why that takes so much CPU usage %, though, as the thread sleeps mostly.

Edit: According to callgrind, most time is spent inside CAddrMan::Select(int), calling CAddrMan::Select_(int), calling GetRandInt(int).

Is that random function so expensive? I think so, after changing to simply use rand(), putting at the top of addrman.cpp a hack

#define GetRandInt(x) (rand()%(x))

The CPU usage dropped back to 4% only. Still significant, but not close to holding up a core. Using rand might be good enough here, as there is no need for cryptographically secure number here. See also issue #1057.

Anyway, using something like exponential backoff in the connection thread is probably the real fix. Just wanted to know why peer selection was so expensive.

@laanwj
Copy link
Member

laanwj commented Sep 19, 2012

Is this fixed in 0.7.0?

@dooglus
Copy link
Contributor Author

dooglus commented Sep 19, 2012

It doesn't use 100% CPU, but it doesn't work either - it still tries to connect:

(gdb) run -server -nolisten -noconnect
[...]
Bitcoin version v0.7.0rc3-dirty-beta (2012-09-12 14:07:22 -0400)
[...]
trying connection 92.240.248.99:8333 lastseen=2.7hrs
connected 92.240.248.99:8333

@dooglus
Copy link
Contributor Author

dooglus commented Sep 19, 2012

I see Pieter's commit fixing the 100% CPU bug in the 0.7.0 code:

commit f161a2c2114cd7d950248ce75a91ba1923e9abb1
Author: Pieter Wuille <pieter.wuille@gmail.com>
Date:   Tue Aug 21 17:32:04 2012 +0200

    Fix infinite loops in connection logic

but nothing implementing "-noconnect" (which apparently was never intended to work anyway).

@sipa
Copy link
Member

sipa commented Sep 19, 2012

Since -connect=X means "connect only to X", -noconnect would mean "don't just connect to one peer", which is exactly what it does :)

Ok, I admit, it wasn't ever considered.

@laanwj
Copy link
Member

laanwj commented Sep 21, 2012

-noconnect makes no sense. But aren't there other ways already to say "don't connect to any peers", or limit the number of outgoing connections?

@dooglus
Copy link
Contributor Author

dooglus commented Sep 21, 2012

I've used "-connect=127.0.0.1" in the past to get no connections.

I'm wondering whether "-maxconnections=0" would work too:

Starting program: /home/chris/Bin/bitcoin -maxconnections=0
[...]
Bitcoin version v0.7.0rc3-dirty-beta (2012-09-12 14:07:22 -0400)
[...]
send version message: version 60002, blocks=199858, us=0.0.0.0:0, them=0.0.0.0:0, peer=127.0.0.1:0
[...]
CTxMemPool::accept() : accepted 058cdc9c4d (poolsz 1)
[...]
Added 4 addresses from 178.18.90.41: 1646 tried, 12503 new
Added 4 addresses from 87.155.86.94: 1646 tried, 12506 new
Added 4 addresses from 173.242.112.53: 1646 tried, 12507 new
[...]
Added 3 addresses from ::: 1646 tried, 12508 new
94 addresses found from DNS seeds
ThreadDNSAddressSeed exited

The GUI says "0 active connections" but the log (above) appears to show some communication with peers, so I'm a bit confused about what's going on there.

@dooglus
Copy link
Contributor Author

dooglus commented Sep 21, 2012

In particular, my client somehow received notification of a payment to one of my addresses (not a payment to self) while I was running with "-maxconnections=0". How can that happen?

CTxMemPool::accept() : accepted 058cdc9c4d (poolsz 1)

When I try sending a payment it doesn't send, as expected:

Status: 0/unconfirmed, has not been successfully broadcast yet
Date: 12-09-21 09:22
To: satoshidice lessthan 8000 1dice6YgEVBf88erBFra9BHf6ZMoyvG88

but for some reason I was able to receive a payment.

@laanwj
Copy link
Member

laanwj commented Oct 26, 2013

The CPU issue was solved, and mapMultiArgs["-connect"].size() > 0 was added, so closing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants