Should fix issues such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/671910152#L7034
test: Properly raise FailedToStartError when rpc shutdown before warmup finished #18561
pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2004-qaFailedToStartConnectionReset changing 1 files +4 −0-
MarcoFalke commented at 1:46 PM on April 8, 2020: member
-
test: Properly raise FailedToStartError when rpc shutdown before warmup finished faede1b293
- fanquake added the label Tests on Apr 8, 2020
-
promag commented at 11:45 AM on April 9, 2020: member
I think connection reset indicates that the server is not properly closing the connection - I suspect the header
Connection: closedoesn't reach the client (?) - so this might be some race/corner-case with libevent where the socket is closed too soon?This doesn't invalidate this change since the retry attempts is limited.
-
MarcoFalke commented at 12:18 PM on April 9, 2020: member
I suspect the header Connection: close doesn't reach the client (?)
Why not? Wouldn't it throw the same exception if it reached the client?
-
promag commented at 12:25 PM on April 9, 2020: member
No, if the header reached the client then it would gracefully close, ECONNRESET means the server closed unexpectedly.
-
MarcoFalke commented at 12:49 PM on April 9, 2020: member
It has to throw some kind of exception if the connection is closed without returning a result (json or jsonexception), no?
-
promag commented at 1:08 PM on April 9, 2020: member
Yes, but why (or in what case) would it close without returning a result? I mean, once the server accepts the connection, it should send the reply.
-
MarcoFalke commented at 1:12 PM on April 9, 2020: member
Yes, but why (or in what case) would it close without returning a result? I mean, once the server accepts the connection, it should send the reply.
Right. This should probably be fixed in libevent or Bitcoin Core. The patch in this pull is to get the tests work around this issue in the meantime.
-
promag commented at 1:14 PM on April 9, 2020: member
The patch in this pull is to get the tests work around this issue in the meantime.
Yeah I agree with this. I can't reproduce but looking at travis error this looks enough.
ACK for that reason.
-
MarcoFalke commented at 1:18 PM on April 9, 2020: member
It happens only in valgrind, so a race seems most likely
- MarcoFalke merged this on Apr 9, 2020
- MarcoFalke closed this on Apr 9, 2020
- MarcoFalke deleted the branch on Apr 9, 2020
- MarcoFalke referenced this in commit b690b24eb2 on Apr 19, 2020
- sidhujag referenced this in commit 0d241c80d4 on Apr 20, 2020
- jasonbcox referenced this in commit c529d04931 on Jul 30, 2020
- DrahtBot locked this on Feb 15, 2022
- vijaydasmp referenced this in commit 3a3f41c119 on Aug 17, 2022
- vijaydasmp referenced this in commit 62a6057245 on Aug 26, 2022
- vijaydasmp referenced this in commit b547c8ea39 on Aug 26, 2022
- vijaydasmp referenced this in commit bdc161d439 on Aug 26, 2022
- vijaydasmp referenced this in commit 2392543a1c on Aug 29, 2022
- vijaydasmp referenced this in commit d43d689fcc on Sep 1, 2022
- vijaydasmp referenced this in commit b1e16994d3 on Sep 3, 2022
- vijaydasmp referenced this in commit 2e7f28bb89 on Sep 4, 2022
- vijaydasmp referenced this in commit b8e5a7c134 on Sep 4, 2022
- vijaydasmp referenced this in commit f6f8bca75a on Sep 5, 2022
- vijaydasmp referenced this in commit 1794841e7e on Sep 8, 2022
- vijaydasmp referenced this in commit 81eda1fb7c on Sep 9, 2022
- vijaydasmp referenced this in commit f4d1a7e542 on Sep 11, 2022
- vijaydasmp referenced this in commit 969159a5d8 on Sep 11, 2022
- vijaydasmp referenced this in commit 2e28e6eee3 on Sep 16, 2022
- ogabrielides referenced this in commit 681a4f7022 on Sep 26, 2022
- michael-EA referenced this in commit 6f22bb4222 on Oct 22, 2022
