Skip to content

Commit 3500c37

Browse files
authored
[PSL-1229] pasteld: handle keep-alive connections (#260)
* [PSL-1229] pasteld: handle keep-alive connections pasteld v2.2.6 Added separate class to keep information about HTTP Connection. Now requests coming from the existing keep-alive http connection create only HTTPRequest object without connection info. HTTPRequest object lifetime is now limited to http_request_cb callback. Save http connections only in unordered_map with client socket as a key (CHttpWorkerContext). - added accept error handler. - improved logging other changes: - returned c++20/gcc-11 changes for Linux - added Ctrl-C console handler for Windows to shutdown pasteld - improved shutdown when pasteld is still in initialization mode (added some checks to exit earlier) * pasteld v2.2.6-beta2: - fixed race condition on accepting new connections * pasteld v2.2.6-beta3 Enhanced the synchronization and notification handling mechanisms in the CHttpWorkerContext class to prevent potential race conditions, ensure robust operation of the event loop, avoid indefinite waits that could lead to deadlocks, and handle potential failures of event_base_loopbreak. Key Changes: - Condition Variable Notifications: Replaced m_LoopCond.notify_one() with m_LoopCond.notify_all() to ensure that all waiting threads are notified of changes to the event loop state. This is critical in scenarios where multiple threads might be waiting for the same condition. - Mutex Locking During Notification: Ensured that m_LoopMutex is locked while calling m_LoopCond.notify_all(). This prevents race conditions by ensuring that notifications and state changes are properly synchronized. - Timed Wait for Condition Variable: Changed the indefinite wait on m_LoopCond.wait to a timed wait using m_LoopCond.wait_for. This avoids the risk of threads waiting indefinitely, which can lead to deadlocks, and allows periodic checks for the event loop state and shutdown conditions. - Handling Potential Failures of event_base_loopbreak: Added logic to repeatedly call event_base_loopbreak in a loop to ensure that the event base loop actually exits, handling cases where event_base_loopbreak might fail on the first call.
1 parent c18a517 commit 3500c37

File tree

5 files changed

+45
-4
lines changed

5 files changed

+45
-4
lines changed

Makefile.am

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ analyze:
126126
test:
127127
./qa/test-suite/full_test_suite.py $(filter-out $@,$(MAKECMDGOALS))
128128

129+
dump:
130+
objdump --disassemble src/pasteld | c++filt > pasteld_dump.txt
131+
129132
.INTERMEDIATE: $(COVERAGE_INFO)
130133

131134
DISTCHECK_CONFIGURE_FLAGS = --enable-man

configure.ac

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ AC_PREREQ([2.60])
33
define(_CLIENT_VERSION_MAJOR, 2)
44
define(_CLIENT_VERSION_MINOR, 2)
55
define(_CLIENT_VERSION_REVISION, 6)
6-
define(_CLIENT_VERSION_BUILD, 0)
6+
define(_CLIENT_VERSION_BUILD, 2)
77
define(_ZC_BUILD_VAL,
88
m4_if(m4_eval(_CLIENT_VERSION_BUILD < 25), 1, m4_incr(_CLIENT_VERSION_BUILD),
99
m4_eval(_CLIENT_VERSION_BUILD < 50), 1,

src/clientversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
#define CLIENT_VERSION_MAJOR 2
1818
#define CLIENT_VERSION_MINOR 2
1919
#define CLIENT_VERSION_REVISION 6
20-
#define CLIENT_VERSION_BUILD 0
20+
#define CLIENT_VERSION_BUILD 2
2121

2222
//! Set to true for release, false for prerelease or test build
2323
#define CLIENT_VERSION_IS_RELEASE true

src/httpserver.cpp

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <thread>
1313
#include <functional>
1414
#include <atomic>
15+
#include <chrono>
1516

1617
#include <compat.h>
1718

@@ -44,6 +45,7 @@
4445
#endif
4546

4647
using namespace std;
48+
using namespace std::chrono_literals;
4749

4850
unique_ptr<CHTTPServer> gl_HttpServer;
4951

@@ -719,6 +721,7 @@ CHttpWorkerContext::CHttpWorkerContext(const size_t nWorkerID) noexcept :
719721
m_base(nullptr),
720722
m_http(nullptr),
721723
m_bEvent(false),
724+
m_bInEventLoop(false),
722725
m_nWorkerID(nWorkerID),
723726
CServiceThread(strprintf("httpevloop%zu", nWorkerID).c_str())
724727
{}
@@ -768,9 +771,29 @@ void CHttpWorkerContext::execute()
768771
{
769772
while (event_base_got_exit(m_base) == 0)
770773
{
774+
{
775+
unique_lock<mutex> lock(m_LoopMutex);
776+
m_bInEventLoop = true;
777+
m_LoopCond.notify_all();
778+
}
779+
771780
event_base_loop(m_base, EVLOOP_NO_EXIT_ON_EMPTY);
781+
772782
if (event_base_got_break(m_base))
783+
{
784+
{
785+
unique_lock<mutex> lock(m_LoopMutex);
786+
m_bInEventLoop = false;
787+
m_LoopCond.notify_all();
788+
}
789+
773790
WaitForEvent();
791+
}
792+
}
793+
{
794+
unique_lock<mutex> lock(m_LoopMutex);
795+
m_bInEventLoop = false;
796+
m_LoopCond.notify_all();
774797
}
775798
LogPrint("http", "[%s] event loop exiting\n", m_sLoopName);
776799
}
@@ -833,8 +856,19 @@ void CHttpWorkerContext::AddHttpConnection(http_connection_t &&pHttpConnection)
833856
}
834857

835858
// break worker event loop to process new http connection
836-
event_base_loopbreak(m_base);
837-
this_thread::yield();
859+
while (m_bInEventLoop && !shouldStop())
860+
{
861+
event_base_loopbreak(m_base);
862+
unique_lock<mutex> lock(m_LoopMutex);
863+
bool bRes = m_LoopCond.wait_for(lock, 100ms, [this] { return !m_bInEventLoop.load() || shouldStop(); });
864+
if (bRes)
865+
break;
866+
}
867+
if (shouldStop())
868+
{
869+
LogPrint("http", "[%s] Cannot process new HTTP connection - shutting down\n", m_sLoopName);
870+
return;
871+
}
838872
evhttp_get_request(m_http, clientSocket, &addr, addrlen);
839873

840874
// notify the worker event loop that it can enter the event loop again

src/httpserver.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ class CHttpWorkerContext : public CServiceThread
182182
evhttp* m_http; // HTTP server
183183
std::string m_sLoopName; // name of the event loop
184184
std::atomic_bool m_bEvent;
185+
std::atomic_bool m_bInEventLoop;
186+
187+
CWaitableCriticalSection m_LoopMutex;
188+
CConditionVariable m_LoopCond;
185189

186190
CWaitableCriticalSection m_mutex;
187191
CConditionVariable m_cond;

0 commit comments

Comments
 (0)