-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
The Issue
When one subscribes to streams using admin command with delivery to url (webhooks), if any of the URL's return HTTP error codes (eg 500, 404) the deliverey queue entries aren't released, resulting in the server stopping to send HTTP subscription deliveries to all endpoints.
Details
rippled/src/xrpld/rpc/detail/RPCSub.cpp
Lines 95 to 142 in db2734c
| void | |
| sendThread() | |
| { | |
| Json::Value jvEvent; | |
| bool bSend; | |
| do | |
| { | |
| { | |
| // Obtain the lock to manipulate the queue and change sending. | |
| std::lock_guard sl(mLock); | |
| if (mDeque.empty()) | |
| { | |
| mSending = false; | |
| bSend = false; | |
| } | |
| else | |
| { | |
| auto const [seq, env] = mDeque.front(); | |
| mDeque.pop_front(); | |
| jvEvent = env; | |
| jvEvent["seq"] = seq; | |
| bSend = true; | |
| } | |
| } | |
| // Send outside of the lock. | |
| if (bSend) | |
| { | |
| // XXX Might not need this in a try. | |
| try | |
| { | |
| JLOG(j_.info()) << "RPCCall::fromNetwork: " << mIp; | |
| RPCCall::fromNetwork( | |
| m_io_context, mIp, mPort, mUsername, mPassword, mPath, "event", jvEvent, mSSL, true, logs_); | |
| } | |
| catch (std::exception const& e) | |
| { | |
| JLOG(j_.info()) << "RPCCall::fromNetwork exception: " << e.what(); | |
| } | |
| } | |
| } while (bSend); | |
| } |
(Note: problem may exist in other parts of code (?) - but the problem is clear)
When RPCCall::fromNetwork() is called at line 134 and receives an HTTP error (500, 404, etc.), an exception is thrown and caught at line 136-138. However:
mSending flag is never reset to false - The flag remains true, indicating the sending thread is still running
No new sending job can be started - The next time send() is called (line 72), it checks if (!mSending) and skips creating a new job since mSending is still true
Events queue up indefinitely - New webhook events get stuck in mDeque with no way to send them
All slots are consumed - This cascades across multiple subscriptions, eventually saturating all available outgoing connection slots
Fix
Clean up the entry/sending state on HTTP error
try
{
JLOG(j_.info()) << "RPCCall::fromNetwork: " << mIp;
RPCCall::fromNetwork(
m_io_context, mIp, mPort, mUsername, mPassword, mPath, "event", jvEvent, mSSL, true, logs_);
}
catch (std::exception const& e)
{
JLOG(j_.info()) << "RPCCall::fromNetwork exception: " << e.what();
}
I would assume the mSending = false; needs to be set after catch as well?
Reproduce
Simple: subscribe to e.g. transaction stream (sends a lot, easy to test).
- Serve 50% HTTP 200, and 50% HTTP 500
- See no more webhooks coming in after a little while