Fix deadlock in TlsStream::Close

fixes #10235

diff --git a/icinga-studio/api.cpp b/icinga-studio/api.cpp
index a92ecabb0010af338b7708b3c6ec1a584421c4b8..2fd0d78038791181444e0965dd87cc4c645f7f59 100644 (file)
--- a/icinga-studio/api.cpp
+++ b/icinga-studio/api.cpp
@@ -54,33 +54,32 @@ void ApiClient::TypesHttpCompletionCallback(HttpRequest& request, HttpResponse&
        while ((count = response.ReadBody(buffer, sizeof(buffer))) > 0)
                body += String(buffer, buffer + count);
 
+       std::vector<ApiType::Ptr> types;
+
        if (response.StatusCode < 200 || response.StatusCode > 299) {
                Log(LogCritical, "ApiClient")
                    <<  "Failed HTTP request; Code: " << response.StatusCode << "; Body: " << body;
-               return;
-       }
+       } else {
+               try {
+                       result = JsonDecode(body);
 
-       std::vector<ApiType::Ptr> types;
+                       Array::Ptr results = result->Get("results");
 
-       try {
-               result = JsonDecode(body);
-
-               Array::Ptr results = result->Get("results");
-
-               ObjectLock olock(results);
-               BOOST_FOREACH(const Dictionary::Ptr typeInfo, results)
-               {
-                       ApiType::Ptr type = new ApiType();;
-                       type->Abstract = typeInfo->Get("abstract");
-                       type->BaseName = typeInfo->Get("base");
-                       type->Name = typeInfo->Get("name");
-                       type->PluralName = typeInfo->Get("plural_name");
-                       // TODO: attributes
-                       types.push_back(type);
+                       ObjectLock olock(results);
+                       BOOST_FOREACH(const Dictionary::Ptr typeInfo, results)
+                       {
+                               ApiType::Ptr type = new ApiType();;
+                               type->Abstract = typeInfo->Get("abstract");
+                               type->BaseName = typeInfo->Get("base");
+                               type->Name = typeInfo->Get("name");
+                               type->PluralName = typeInfo->Get("plural_name");
+                               // TODO: attributes
+                               types.push_back(type);
+                       }
+               } catch (const std::exception& ex) {
+                       Log(LogCritical, "ApiClient")
+                           << "Error while decoding response: " << DiagnosticInformation(ex);
                }
-       } catch (const std::exception& ex) {
-               Log(LogCritical, "ApiClient")
-                   << "Error while decoding response: " << DiagnosticInformation(ex);
        }
 
        callback(types);
@@ -125,54 +124,54 @@ void ApiClient::ObjectsHttpCompletionCallback(HttpRequest& request,
        while ((count = response.ReadBody(buffer, sizeof(buffer))) > 0)
                body += String(buffer, buffer + count);
 
+       std::vector<ApiObject::Ptr> objects;
+
        if (response.StatusCode < 200 || response.StatusCode > 299) {
                Log(LogCritical, "ApiClient")
                    <<  "Failed HTTP request; Code: " << response.StatusCode << "; Body: " << body;
                return;
-       }
+       } else {
+               try {
+                       result = JsonDecode(body);
 
-       std::vector<ApiObject::Ptr> objects;
-
-       try {
-               result = JsonDecode(body);
-
-               Array::Ptr results = result->Get("results");
+                       Array::Ptr results = result->Get("results");
 
-               if (results) {
-                       ObjectLock olock(results);
-                       BOOST_FOREACH(const Dictionary::Ptr objectInfo, results)
-                       {
-                               ApiObject::Ptr object = new ApiObject();
+                       if (results) {
+                               ObjectLock olock(results);
+                               BOOST_FOREACH(const Dictionary::Ptr objectInfo, results)
+                               {
+                                       ApiObject::Ptr object = new ApiObject();
 
-                               Dictionary::Ptr attrs = objectInfo->Get("attrs");
+                                       Dictionary::Ptr attrs = objectInfo->Get("attrs");
 
-                               {
-                                       ObjectLock olock(attrs);
-                                       BOOST_FOREACH(const Dictionary::Pair& kv, attrs)
                                        {
-                                               object->Attrs[kv.first] = kv.second;
+                                               ObjectLock olock(attrs);
+                                               BOOST_FOREACH(const Dictionary::Pair& kv, attrs)
+                                               {
+                                                       object->Attrs[kv.first] = kv.second;
+                                               }
                                        }
-                               }
 
-                               Array::Ptr used_by = objectInfo->Get("used_by");
+                                       Array::Ptr used_by = objectInfo->Get("used_by");
 
-                               {
-                                       ObjectLock olock(used_by);
-                                       BOOST_FOREACH(const Dictionary::Ptr& refInfo, used_by)
                                        {
-                                               ApiObjectReference ref;
-                                               ref.Name = refInfo->Get("name");
-                                               ref.Type = refInfo->Get("type");
-                                               object->UsedBy.push_back(ref);
+                                               ObjectLock olock(used_by);
+                                               BOOST_FOREACH(const Dictionary::Ptr& refInfo, used_by)
+                                               {
+                                                       ApiObjectReference ref;
+                                                       ref.Name = refInfo->Get("name");
+                                                       ref.Type = refInfo->Get("type");
+                                                       object->UsedBy.push_back(ref);
+                                               }
                                        }
-                               }
 
-                               objects.push_back(object);
+                                       objects.push_back(object);
+                               }
                        }
+               } catch (const std::exception& ex) {
+                       Log(LogCritical, "ApiClient")
+                               << "Error while decoding response: " << DiagnosticInformation(ex);
                }
-       } catch (const std::exception& ex) {
-               Log(LogCritical, "ApiClient")
-                       << "Error while decoding response: " << DiagnosticInformation(ex);
        }
 
        callback(objects);

diff --git a/lib/base/socketevents.cpp b/lib/base/socketevents.cpp
index 8c420e7090e543abf5c07355384c7069ea1b88c6..b86d9a5c7f7bdcc03a973c23af0d73b9f0f09bbe 100644 (file)
--- a/lib/base/socketevents.cpp
+++ b/lib/base/socketevents.cpp
@@ -166,10 +166,12 @@ void SocketEvents::WakeUpThread(bool wait)
 
                        l_SocketIOFDChanged = true;
 
-                       (void) send(l_SocketIOEventFDs[1], "T", 1, 0);
+                       while (l_SocketIOFDChanged) {
+                               (void) send(l_SocketIOEventFDs[1], "T", 1, 0);
 
-                       while (l_SocketIOFDChanged)
-                               l_SocketIOCV.wait(lock);
+                               boost::system_time const timeout = boost::get_system_time() + boost::posix_time::milliseconds(50);
+                               l_SocketIOCV.timed_wait(lock, timeout);
+                       }
                }
        } else {
                (void) send(l_SocketIOEventFDs[1], "T", 1, 0);

diff --git a/lib/base/stream.cpp b/lib/base/stream.cpp
index 524cd07821dbe99115ab5fa3dab5513016a68d7f..cfcdf0bb05a86d8a90e49101aa2d2dab75e05a84 100644 (file)
--- a/lib/base/stream.cpp
+++ b/lib/base/stream.cpp
@@ -22,7 +22,7 @@
 
 using namespace icinga;
 
-void Stream::RegisterDataHandler(const boost::function<void(void)>& handler)
+void Stream::RegisterDataHandler(const boost::function<void(const Stream::Ptr&)>& handler)
 {
        if (SupportsWaiting())
                OnDataAvailable.connect(handler);
@@ -52,7 +52,7 @@ size_t Stream::Peek(void *buffer, size_t count, bool allow_partial)
 
 void Stream::SignalDataAvailable(void)
 {
-       OnDataAvailable();
+       OnDataAvailable(this);
 
        {
                boost::mutex::scoped_lock lock(m_Mutex);

diff --git a/lib/base/stream.hpp b/lib/base/stream.hpp
index 1131d64083b653372dc3d227caea1a66d6b1a2be..32ed4bb8d0b3b1e176d634e14715572c746be23e 100644 (file)
--- a/lib/base/stream.hpp
+++ b/lib/base/stream.hpp
@@ -131,7 +131,7 @@ public:
 
        virtual bool IsDataAvailable(void) const;
 
-       void RegisterDataHandler(const boost::function<void(void)>& handler);
+       void RegisterDataHandler(const boost::function<void(const Stream::Ptr&)>& handler);
 
        StreamReadStatus ReadLine(String *line, StreamReadContext& context, bool may_wait = false);
 
@@ -139,7 +139,7 @@ protected:
        void SignalDataAvailable(void);
 
 private:
-       boost::signals2::signal<void(void)> OnDataAvailable;
+       boost::signals2::signal<void(const Stream::Ptr&)> OnDataAvailable;
 
        boost::mutex m_Mutex;
        boost::condition_variable m_CV;

diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp
index 9bcb5c6ebe10757ff20950a768731784ac542d43..ba0333b258484e9105d10a7d744568e637aa7c64 100644 (file)
--- a/lib/base/tlsstream.cpp
+++ b/lib/base/tlsstream.cpp
@@ -184,10 +184,12 @@ void TlsStream::OnEvent(int revents)
        if (rc > 0) {
                m_CurrentAction = TlsActionNone;
 
-               if (m_SendQ->GetAvailableBytes() > 0)
-                       ChangeEvents(POLLIN|POLLOUT);
-               else
-                       ChangeEvents(POLLIN);
+               if (!m_Eof) {
+                       if (m_SendQ->GetAvailableBytes() > 0)
+                               ChangeEvents(POLLIN|POLLOUT);
+                       else
+                               ChangeEvents(POLLIN);
+               }
 
                lock.unlock();
 

diff --git a/lib/remote/httpclientconnection.cpp b/lib/remote/httpclientconnection.cpp
index e6688df71a5268867e9629e91ea04c3ec80acae7..bcc834ded41064c18fafd85d6522f5706b34ebb8 100644 (file)
--- a/lib/remote/httpclientconnection.cpp
+++ b/lib/remote/httpclientconnection.cpp
@@ -49,6 +49,9 @@ void HttpClientConnection::Reconnect(void)
        m_Context.~StreamReadContext();
        new (&m_Context) StreamReadContext();
 
+       m_Requests.clear();
+       m_CurrentResponse.reset();
+
        TcpSocket::Ptr socket = new TcpSocket();
        socket->Connect(m_Host, m_Port);
 
@@ -59,9 +62,9 @@ void HttpClientConnection::Reconnect(void)
                /* m_Stream = new NetworkStream(socket);
                   -- does not currently work because the NetworkStream class doesn't support async I/O */
 
-       m_Stream->RegisterDataHandler(boost::bind(&HttpClientConnection::DataAvailableHandler, this));
+       m_Stream->RegisterDataHandler(boost::bind(&HttpClientConnection::DataAvailableHandler, this, _1));
        if (m_Stream->IsDataAvailable())
-               DataAvailableHandler();
+               DataAvailableHandler(m_Stream);
 }
 
 Stream::Ptr HttpClientConnection::GetStream(void) const
@@ -95,8 +98,10 @@ bool HttpClientConnection::ProcessMessage(void)
 {
        bool res;
 
-       if (m_Requests.empty())
+       if (m_Requests.empty()) {
+               m_Stream->Close();
                return false;
+       }
 
        const std::pair<boost::shared_ptr<HttpRequest>, HttpCompletionCallback>& currentRequest = *m_Requests.begin();
        HttpRequest& request = *currentRequest.first.get();
@@ -129,19 +134,26 @@ bool HttpClientConnection::ProcessMessage(void)
        return res;
 }
 
-void HttpClientConnection::DataAvailableHandler(void)
+void HttpClientConnection::DataAvailableHandler(const Stream::Ptr& stream)
 {
        boost::mutex::scoped_lock lock(m_DataHandlerMutex);
 
+       ASSERT(stream == m_Stream);
+
        try {
                while (ProcessMessage())
                        ; /* empty loop body */
        } catch (const std::exception& ex) {
                Log(LogWarning, "HttpClientConnection")
-                   << "Error while reading Http request: " << DiagnosticInformation(ex);
+                   << "Error while reading Http response: " << DiagnosticInformation(ex);
 
                Disconnect();
        }
+
+       if (m_Context.Eof) {
+               Log(LogWarning, "HttpClientConnection", "Encountered unexpected EOF while reading Http response.");
+               m_Stream->Close();
+       }
 }
 
 boost::shared_ptr<HttpRequest> HttpClientConnection::NewRequest(void)

diff --git a/lib/remote/httpclientconnection.hpp b/lib/remote/httpclientconnection.hpp
index 72fcfeef98556438f8b9e4fcebc7b909e95b0215..50bddbc40dd0019d3aefb55f10993454fac481ce 100644 (file)
--- a/lib/remote/httpclientconnection.hpp
+++ b/lib/remote/httpclientconnection.hpp
@@ -68,7 +68,7 @@ private:
 
        void Reconnect(void);
        bool ProcessMessage(void);
-       void DataAvailableHandler(void);
+       void DataAvailableHandler(const Stream::Ptr& stream);
 
        void ProcessMessageAsync(HttpRequest& request);
 };

diff --git a/lib/remote/httpresponse.cpp b/lib/remote/httpresponse.cpp
index f129f51c754608738adb38c10fafcad4cfdcb4f9..c5c9c722a328f134140d58e0a76df75a5f9db3cb 100644 (file)
--- a/lib/remote/httpresponse.cpp
+++ b/lib/remote/httpresponse.cpp
@@ -34,10 +34,14 @@ HttpResponse::HttpResponse(const Stream::Ptr& stream, const HttpRequest& request
 
 void HttpResponse::SetStatus(int code, const String& message)
 {
-       ASSERT(m_State == HttpResponseStart);
        ASSERT(code >= 100 && code <= 599);
        ASSERT(!message.IsEmpty());
 
+       if (m_State != HttpResponseStart) {
+               Log(LogWarning, "HttpResponse", "Tried to set Http response status after headers had already been sent.");
+               return;
+       }
+
        String status = "HTTP/";
 
        if (m_Request.ProtocolVersion == HttpVersion10)
@@ -54,7 +58,11 @@ void HttpResponse::SetStatus(int code, const String& message)
 
 void HttpResponse::AddHeader(const String& key, const String& value)
 {
-       ASSERT(m_State = HttpResponseHeaders);
+       if (m_State != HttpResponseHeaders) {
+               Log(LogWarning, "HttpResponse", "Tried to add header after headers had already been sent.");
+               return;
+       }
+
        String header = key + ": " + value + "\r\n";
        m_Stream->Write(header.CStr(), header.GetLength());
 }

diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp
index 4348be986caf43b66a7ec5b3f01fa28954f89178..13f321df4d410081b5d759dfaac2ecb23d75ca02 100644 (file)
--- a/lib/remote/httpserverconnection.cpp
+++ b/lib/remote/httpserverconnection.cpp
@@ -157,6 +157,8 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request)
                try {
                        HttpHandler::ProcessRequest(user, request, response);
                } catch (const std::exception& ex) {
+                       Log(LogCritical, "HttpServerConnection")
+                           << "Unhandled exception while processing Http request: " << DiagnosticInformation(ex);
                        response.SetStatus(503, "Unhandled exception");
                        response.AddHeader("Content-Type", "text/plain");
                        String errorInfo = DiagnosticInformation(ex);