Expert needed: Multithreading JSON-RPC

[Luke-Jr]

I wrote a patch to give JSON-RPC connections their own threads. However, it still has some issues that I can't seem to figure out. If anyone can advise on how to fix it, this would be a nice feature.

• Occasional crashes (SIGABRT)
• Segfault if -DUSE_SSL
• Timed out connections aren't closed

http://luke.dashjr.org/programs/bitcoin/w/bitcoind/luke-jr.git/commitdiff/355e162fba35810978742f6f1e6f2e413cdc78e9

[1713889527]

1713889527

[xf2_org]

Cute, but definitely not that easy.  If it was, we would have already done that by now.  There are way too many "there is only one RPC thread" assumptions built into the code at the present time.

[Luke-Jr]

Well, care to help? I basically "need" this functionality to setup my pool how I want it-- a 'getwork' request depends on being able to make sub-RPC calls.

Here's a new version, that mostly works fine without -DUSE_SSL... Though I can still crash it if I keep a constant load of 100+ RPC calls at once, it's probably "stable enough" for my pool to just restart it when it dies.

http://luke.dashjr.org/programs/bitcoin/w/bitcoind/luke-jr.git/commitdiff/0071352b3347f92faa266dfd1db18aea88b45c54

[Luke-Jr]

What if the connection threads grabbed a lock, and the "real" multithreading only occurs when the RPC function itself releases that lock?

[xf2_org]

It's such a mess that I thought about ripping out boost SSL, and doing threading with straight OpenSSL C API.  That eliminates a few of the assumptions, but not all by a long shot.

If you really do manage to fix them all, your pull request will get merged faster than you can say "tonal"

[Luke-Jr]

Well, locking helped the non-SSL code. I can no longer crash it. Not sure I care enough to figure out the SSL problem.

[jordanlewis]

As I understand it, it's not actually all that beneficial to spin off a new request thread for every incoming connection, at least in standard webservers. The general approach is to have a few worker threads processing requests, but each of them asynchronously. So one thread would be able to handle a few incoming connections at once by removing the sequential connection processing code.

I've had a look at the RPC server code and it doesn't seem like it would be that hard to get basic asynchronous IO going, which would help solve any timeout issues by itself. Of course you'd still need to put a lock around all of the actual bitcoin calls until that "one rpc thread" assumption is removed, but nonetheless I think it would be an improvement.

Does that sound like it could be a helpful patch? I'd like to get some feedback before doing any more work in this direction.

[jordanlewis]

I forgot to mention that the code already uses Boost/ASIO enough that making the RPC server code asynchronous is a pretty natural direction to go in.

[Luke-Jr]

Asynchronous wouldn't solve it for me. I have JSON-RPC requests blocking on loopback JSON-RPC requests. My current code is working so long as I build without SSL support, so I guess it's good enough for now.

[xf2_org]

As I understand it, it's not actually all that beneficial to spin off a new request thread for every incoming connection, at least in standard webservers. The general approach is to have a few worker threads processing requests, but each of them asynchronously. So one thread would be able to handle a few incoming connections at once by removing the sequential connection processing code.

I've had a look at the RPC server code and it doesn't seem like it would be that hard to get basic asynchronous IO going, which would help solve any timeout issues by itself. Of course you'd still need to put a lock around all of the actual bitcoin calls until that "one rpc thread" assumption is removed, but nonetheless I think it would be an improvement.

Does that sound like it could be a helpful patch? I'd like to get some feedback before doing any more work in this direction.

Definitely!  We would love to see a patch that updated the RPC HTTP server to use async I/O.