Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented May 7, 2014

Some commits to (hopefully) improve RPC behaviour:

  • Pass errors from async_accept
  • Make sure connection object is always cleaned up
    See individual commit messages for details.
  • Make sure that acceptors and timers are cancelled before exiting the event loops.

@laanwj laanwj added this to the 0.9.2 milestone May 9, 2014
@laanwj laanwj added the RPC label May 9, 2014
laanwj added 4 commits May 12, 2014 09:30
According to the [boost::asio documentation](http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference/basic_socket_acceptor/async_accept/overload2.html),
the function signature of the handler must be:

    void handler(
      const boost::system::error_code& error // Result of operation.
    );

We were binding *all* the arguments, instead of all but the error,
resulting in nullary function that never got the error. Fix this
by adding an input argument substitution.
Make sure conn object always gets cleaned up by using a
`boost::shared_ptr`.

This makes valgrind happy - before this commit, one connection object
always leaked at shutdown, as well as can avoid other leaks, when
for example an exception happens.

Also add an explicit Close() to the !ClientAllowed path to make it similar
to the normal path (I'm not sure whether it is needed, but it
can't hurt).
That option hasn't existed for a long time.
Fixes bitcoin#4156.

The problem is that the boost::asio::io_service destructor
waits for the acceptors to finish (on windows, and boost 1.55).

Fix this by keeping track of the acceptors and cancelling them before
stopping the event loops.
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/cef44941e798c33f7334bf90a2dd531f9bada8c3 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@laanwj laanwj merged commit cef4494 into bitcoin:master May 12, 2014
laanwj added a commit that referenced this pull request May 12, 2014
cef4494 rpc: keep track of acceptors, and cancel them in StopRPCThreads (Wladimir J. van der Laan)
381b25d doc: remove mention of `-rpctimeout` from man page (Wladimir J. van der Laan)
1a44522 rpc: Make sure conn object is always cleaned up (Wladimir J. van der Laan)
0a0cd34 rpc: pass errors from async_accept (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants