Aligned main trunk with cache development

Sebastiano Merlino · Sebastiano Merlino · commit 7681d9677830 · 2012-12-26T22:15:30.000+01:00
diff --git a/src/http_response.cpp b/src/http_response.cpp
@@ -37,7 +37,7 @@ void unlock_on_close::do_action()
 
 cache_response::~cache_response()
 {
-    if(ce != 0x0 && locked_element)
+    if(ce != 0x0)
         details::unlock_cache_entry(ce);
 }
 
@@ -164,13 +164,12 @@ void http_file_response::get_raw_response(MHD_Response** response, webserver* ws
 
 void cache_response::get_raw_response(MHD_Response** response, webserver* ws)
 {
-    this->locked_element = true;
     bool valid;
     http_response* r;
     if(ce == 0x0)
         r = ws->get_from_cache(content, &valid, &ce, true, false);
     else
-        r = details::get_response(ce);
+        details::get_response(ce, &r);
     r->get_raw_response(response, ws);
     r->decorate_response(*response); //It is done here to avoid to search two times for the same element
     
diff --git a/src/httpserver/http_response.hpp b/src/httpserver/http_response.hpp
@@ -137,7 +137,8 @@ class http_response
             const std::vector<std::string>& topics = std::vector<std::string>(),
             int keepalive_secs = -1,
             const std::string keepalive_msg = "",
-            const std::string send_topic = ""
+            const std::string send_topic = "",
+            cache_entry* ce = 0x0
         ):
             response_type(response_type),
             content(content),
@@ -152,7 +153,8 @@ class http_response
             keepalive_secs(keepalive_secs),
             keepalive_msg(keepalive_msg),
             send_topic(send_topic),
-            ca(new closure_action())
+            ca(0x0),
+            ce(ce)
         {
             set_header(http_utils::http_header_content_type, content_type);
         }
@@ -177,9 +179,9 @@ class http_response
             keepalive_secs(b.keepalive_secs),
             keepalive_msg(b.keepalive_msg),
             send_topic(b.send_topic),
-            ca(new closure_action())
+            ca(0x0),
+            ce(b.ce)
         {
-            *ca = b.ca;
         }
         http_response& operator=(const http_response& b)
         {
@@ -205,6 +207,7 @@ class http_response
                 ca = 0x0;
             }
             ca = b.ca;
+            ce = b.ce;
             return *this;
         }
         virtual ~http_response()
@@ -441,6 +444,7 @@ class http_response
         std::string send_topic;
         struct MHD_Connection* underlying_connection;
         closure_action* ca;
+        cache_entry* ce;
 
         virtual void get_raw_response(MHD_Response** res, webserver* ws = 0x0);
         virtual void decorate_response(MHD_Response* res);
@@ -630,20 +634,14 @@ class cache_response : public http_response
     public:
         cache_response
         (
-            const std::string& key,
-            bool locked_element = false
-        ) : http_response(http_response::CACHED_CONTENT, key),
-            ce(0x0),
-            locked_element(locked_element)
+            const std::string& key
+        ) : http_response(http_response::CACHED_CONTENT, key, 200, "", true, "", "", false, std::vector<std::string>(), -1, "", "", 0x0)
         {
         }
         cache_response
         (
-            cache_entry* ce,
-            bool locked_element = false
-        ) : http_response(http_response::CACHED_CONTENT, ""),
-            ce(ce),
-            locked_element(locked_element)
+            cache_entry* ce
+        ) : http_response(http_response::CACHED_CONTENT, "", 200, "", true, "", "", false, std::vector<std::string>(), -1, "", "", ce)
         {
             if(ce == 0x0)
                 throw bad_caching_attempt();
@@ -656,9 +654,6 @@ class cache_response : public http_response
     protected:
         virtual void get_raw_response(MHD_Response** res, webserver* ws = 0x0);
         virtual void decorate_response(MHD_Response* res);
-    private:
-        cache_entry* ce;
-        bool locked_element;
         friend class webserver;
 };
 
diff --git a/src/httpserver/webserver.hpp b/src/httpserver/webserver.hpp
@@ -65,7 +65,7 @@ namespace details
     struct daemon_item;
     void unlock_cache_entry(cache_entry*);
     void lock_cache_entry(cache_entry*);
-    http_response* get_response(cache_entry*);
+    void get_response(cache_entry*, http_response** res);
 }
 
 using namespace http;
@@ -269,7 +269,9 @@ class webserver
         http_response* get_from_cache(const std::string& key, bool* valid, bool lock = false, bool write = false);
         http_response* get_from_cache(const std::string& key, bool* valid, cache_entry** ce, bool lock = false, bool write = false);
         void lock_cache_element(const std::string& key, bool write = false);
+        void lock_cache_element(cache_entry* ce, bool write = false);
         void unlock_cache_element(const std::string& key);
+        void unlock_cache_element(cache_entry* ce);
         cache_entry* put_in_cache(const std::string& key, http_response* value, bool* new_elem, bool lock = false, bool write = false, int validity = -1);
         void remove_from_cache(const std::string& key);
         bool is_valid(const std::string& key);
@@ -336,8 +338,8 @@ class webserver
 
         std::map<details::http_endpoint, http_resource* > registered_resources;
 
-        pthread_rwlock_t cache_guard;
         std::map<std::string, cache_entry*> response_cache;
+        pthread_rwlock_t cache_guard;
 #ifdef USE_CPP_ZEROX
         std::unordered_set<ip_representation> bans;
         std::unordered_set<ip_representation> allowances;
diff --git a/src/webserver.cpp b/src/webserver.cpp
@@ -162,6 +162,27 @@ struct modded_request
     http_request* dhr;
     http_response_ptr dhrs;
     bool second;
+
+    modded_request():
+        pp(0x0),
+        complete_uri(0x0),
+        ws(0x0),
+        dhr(0x0),
+        dhrs(0x0),
+        second(false)
+    {
+    }
+    ~modded_request()
+    {
+        if (NULL != pp) 
+        {
+            MHD_destroy_post_processor (pp);
+        }
+        if(second)
+            delete dhr; //TODO: verify. It could be an error
+        delete complete_uri;
+    }
+
 };
 
 }
@@ -173,12 +194,11 @@ struct cache_entry
     details::http_response_ptr response;
     pthread_rwlock_t elem_guard;
     pthread_mutex_t lock_guard;
-    bool locked;
+    set<pthread_t> lockers;
 
     cache_entry():
         ts(-1),
-        validity(-1),
-        locked(false)
+        validity(-1)
     {
         pthread_rwlock_init(&elem_guard, NULL);
         pthread_mutex_init(&lock_guard, NULL);
@@ -195,8 +215,7 @@ struct cache_entry
         validity(b.validity),
         response(b.response),
         elem_guard(b.elem_guard),
-        lock_guard(b.lock_guard),
-        locked(b.locked)
+        lock_guard(b.lock_guard)
     {
     }
 
@@ -208,14 +227,12 @@ struct cache_entry
         pthread_rwlock_destroy(&elem_guard);
         pthread_mutex_destroy(&lock_guard);
         elem_guard = b.elem_guard;
-        locked = b.locked;
     }
 
     cache_entry(details::http_response_ptr response, long ts = -1, int validity = -1):
         ts(ts),
         validity(validity),
-        response(response),
-        locked(false)
+        response(response)
     {
         pthread_rwlock_init(&elem_guard, NULL);
         pthread_mutex_init(&lock_guard, NULL);
@@ -224,24 +241,32 @@ struct cache_entry
     void lock(bool write = false)
     {
         pthread_mutex_lock(&lock_guard);
-        if(!locked)
+        if(!lockers.count(pthread_self()))
         {
             if(write)
+            {
+                lockers.insert(pthread_self());
+                pthread_mutex_unlock(&lock_guard);
                 pthread_rwlock_wrlock(&elem_guard);
+            }
             else
+            {
+                lockers.insert(pthread_self());
+                pthread_mutex_unlock(&lock_guard);
                 pthread_rwlock_rdlock(&elem_guard);
-            locked = true;
+            }
         }
-        pthread_mutex_unlock(&lock_guard);
+        else
+            pthread_mutex_unlock(&lock_guard);
     }
 
     void unlock()
     {
         pthread_mutex_lock(&lock_guard);
-        if(locked)
+        if(lockers.count(pthread_self()))
         {
+            lockers.erase(pthread_self());
             pthread_rwlock_unlock(&elem_guard);
-            locked = false;
         }
         pthread_mutex_unlock(&lock_guard);
     }
@@ -260,9 +285,9 @@ void lock_cache_entry(cache_entry* ce)
     ce->lock();
 }
 
-http_response* get_response(cache_entry* ce)
+void get_response(cache_entry* ce, http_response** res)
 {
-    return ce->response.ptr();
+    *res = ce->response.ptr();
 }
 
 };
@@ -509,7 +534,6 @@ webserver& webserver::operator=(const webserver& b)
     method_not_allowed_resource = b.method_not_allowed_resource;
     method_not_acceptable_resource = b.method_not_acceptable_resource;
     internal_error_resource = b.internal_error_resource;
-    response_cache = b.response_cache;
     return *this;
 }
 
@@ -558,23 +582,12 @@ void webserver::sweet_kill()
 void webserver::request_completed (void *cls, struct MHD_Connection *connection, void **con_cls, enum MHD_RequestTerminationCode toe) 
 {
     details::modded_request* mr = (struct details::modded_request*) *con_cls;
-    if(mr != 0x0 && mr->dhrs.res != 0x0)
+    if (0x0 == mr) 
     {
-        if(mr->dhrs->ca != 0x0)
+        if(mr->dhrs.res != 0x0 && mr->dhrs->ca != 0x0)
             mr->dhrs->ca->do_action();
+        delete mr;
     }
-    if (NULL == mr) 
-    {
-        return;
-    }
-    if (NULL != mr->pp) 
-    {
-        MHD_destroy_post_processor (mr->pp);
-    }
-    if(mr->second)
-        delete mr->dhr; //TODO: verify. It could be an error
-    delete mr->complete_uri;
-    delete mr;
 }
 
 void webserver::schedule_fd(int fd, fd_set* schedule_list, int* max)
@@ -1443,6 +1456,7 @@ int webserver::finalize_answer(MHD_Connection* connection, struct details::modde
     catch(...)
     {
         internal_error_page(&dhrs, mr, true);
+        dhrs->get_raw_response(&raw_response, this);
     }
     dhrs->decorate_response(raw_response);
     to_ret = dhrs->enqueue_response(connection, raw_response);
@@ -1782,8 +1796,18 @@ void webserver::lock_cache_element(const std::string& key, bool write)
     pthread_rwlock_rdlock(&cache_guard);
     map<string, cache_entry*>::iterator it(response_cache.find(key));
     if(it != response_cache.end())
+    {
+        pthread_rwlock_unlock(&cache_guard);
         (*it).second->lock(write);
-    pthread_rwlock_unlock(&cache_guard);
+    }
+    else
+        pthread_rwlock_unlock(&cache_guard);
+}
+
+void webserver::lock_cache_element(cache_entry* ce, bool write)
+{
+    if(ce)
+        ce->lock(write);
 }
 
 void webserver::unlock_cache_element(const std::string& key)
@@ -1795,6 +1819,12 @@ void webserver::unlock_cache_element(const std::string& key)
     pthread_rwlock_unlock(&cache_guard);
 }
 
+void webserver::unlock_cache_element(cache_entry* ce)
+{
+    if(ce)
+        ce->unlock();
+}
+
 cache_entry* webserver::put_in_cache(const std::string& key, http_response* value, bool* new_elem, bool lock, bool write, int validity)
 {
     pthread_rwlock_wrlock(&cache_guard);

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ void unlock_on_close::do_action()`
`37`	`37`
`38`	`38`	`cache_response::~cache_response()`
`39`	`39`	`{`
`40`		`- if(ce != 0x0 && locked_element)`
	`40`	`+ if(ce != 0x0)`
`41`	`41`	`details::unlock_cache_entry(ce);`
`42`	`42`	`}`
`43`	`43`
`@@ -164,13 +164,12 @@ void http_file_response::get_raw_response(MHD_Response** response, webserver* ws`
`164`	`164`
`165`	`165`	`void cache_response::get_raw_response(MHD_Response** response, webserver* ws)`
`166`	`166`	`{`
`167`		`- this->locked_element = true;`
`168`	`167`	`bool valid;`
`169`	`168`	`http_response* r;`
`170`	`169`	`if(ce == 0x0)`
`171`	`170`	`r = ws->get_from_cache(content, &valid, &ce, true, false);`
`172`	`171`	`else`
`173`		`- r = details::get_response(ce);`
	`172`	`+ details::get_response(ce, &r);`
`174`	`173`	`r->get_raw_response(response, ws);`
`175`	`174`	`r->decorate_response(*response); //It is done here to avoid to search two times for the same element`
`176`	`175`