Make check works, no problems (yet) with threads

diff --git a/src/client.c b/src/client.c
index 94414ff..fce605d 100644 (file)
--- a/src/client.c
+++ b/src/client.c
@@ -517,13 +517,9 @@ void client_check_preferred_watch(struct client *cl)
     yaz_log(YLOG_DEBUG, "client_check_preferred_watch: %s ", client_get_id(cl));
     if (se)
     {
-        /* TODO possible threading issue. Session can have been destroyed */
         assert(cl->session);
-        if (session_is_preferred_clients_ready(se)) {
-            assert(cl->session);
+        if (session_is_preferred_clients_ready(se))
             session_alert_watch(se, SESSION_WATCH_SHOW_PREF);
-            assert(cl->session);
-        }
         else
             yaz_log(YLOG_DEBUG, "client_check_preferred_watch: Still locked on preferred targets.");
         assert(cl->session);
@@ -1598,12 +1594,6 @@ int client_get_maxrecs(struct client *cl)
     return cl->maxrecs;
 }
 
-void client_set_preferred(struct client *cl, int v)
-{
-    cl->preferred = v;
-}
-
-
 struct suggestions* client_suggestions_create(const char* suggestions_string)
 {
     int i;

diff --git a/src/ppmutex.c b/src/ppmutex.c
index 8ddb201..2882bac 100644 (file)
--- a/src/ppmutex.c
+++ b/src/ppmutex.c
@@ -43,7 +43,7 @@ void pazpar2_mutex_create(YAZ_MUTEX *p, const char *name)
     yaz_mutex_set_name(*p, ppmutex_level, name);
 }
 
-int pazpar2_lock_rdwr_init(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_init(Pazpar2_lock_rdwr *p)
 {
     p->readers_reading = 0;
     p->writers_writing = 0;
@@ -51,10 +51,9 @@ int pazpar2_lock_rdwr_init(Pazpar2_lock_rdwr *p)
     pthread_mutex_init(&p->mutex, 0);
     pthread_cond_init(&p->lock_free, 0);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_destroy(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_destroy(Pazpar2_lock_rdwr *p)
 {
     assert (p->readers_reading == 0);
     assert (p->writers_writing == 0);
@@ -62,10 +61,9 @@ int pazpar2_lock_rdwr_destroy(Pazpar2_lock_rdwr *p)
     pthread_mutex_destroy(&p->mutex);
     pthread_cond_destroy(&p->lock_free);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_rlock(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_rlock(Pazpar2_lock_rdwr *p)
 {
 #if YAZ_POSIX_THREADS
     pthread_mutex_lock(& p->mutex);
@@ -74,10 +72,9 @@ int pazpar2_lock_rdwr_rlock(Pazpar2_lock_rdwr *p)
     p->readers_reading++;
     pthread_mutex_unlock(&p->mutex);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_wlock(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_wlock(Pazpar2_lock_rdwr *p)
 {
 #if YAZ_POSIX_THREADS
     pthread_mutex_lock(&p->mutex);
@@ -86,12 +83,12 @@ int pazpar2_lock_rdwr_wlock(Pazpar2_lock_rdwr *p)
     p->writers_writing++;
     pthread_mutex_unlock(&p->mutex);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_upgrade(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_upgrade(Pazpar2_lock_rdwr *p)
 {
 #if YAZ_POSIX_THREADS
+    assert(p->readers_reading > 0);
     pthread_mutex_lock(&p->mutex);
     --p->readers_reading;
     while (p->writers_writing || p->readers_reading)
@@ -99,46 +96,41 @@ int pazpar2_lock_rdwr_upgrade(Pazpar2_lock_rdwr *p)
     p->writers_writing++;
     pthread_mutex_unlock(&p->mutex);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_runlock(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_downgrade(Pazpar2_lock_rdwr *p)
 {
 #if YAZ_POSIX_THREADS
+    assert(p->writers_writing == 1);
     pthread_mutex_lock(&p->mutex);
+    p->writers_writing--;
+    p->readers_reading++;
+    pthread_cond_broadcast(&p->lock_free);
+    pthread_mutex_unlock(&p->mutex);
+#endif
+}
+
+void pazpar2_lock_rdwr_runlock(Pazpar2_lock_rdwr *p)
+{
+#if YAZ_POSIX_THREADS
+    assert(p->readers_reading > 0);
+    pthread_mutex_lock(&p->mutex);
+    p->readers_reading--;
     if (p->readers_reading == 0)
-    {
-       pthread_mutex_unlock(&p->mutex);
-       return -1;
-    } 
-    else
-    {
-       p->readers_reading--;
-       if (p->readers_reading == 0)
-           pthread_cond_signal(&p->lock_free);
-       pthread_mutex_unlock(&p->mutex);
-    }
+        pthread_cond_signal(&p->lock_free);
+    pthread_mutex_unlock(&p->mutex);
 #endif
-    return 0;
 }
 
-int pazpar2_lock_rdwr_wunlock(Pazpar2_lock_rdwr *p)
+void pazpar2_lock_rdwr_wunlock(Pazpar2_lock_rdwr *p)
 {
 #if YAZ_POSIX_THREADS
+    assert(p->writers_writing == 1);
     pthread_mutex_lock(&p->mutex);
-    if (p->writers_writing == 0)
-    {
-       pthread_mutex_unlock(&p->mutex);
-       return -1;
-    }
-    else
-    {
-       p->writers_writing--;
-       pthread_cond_broadcast(&p->lock_free);
-       pthread_mutex_unlock(&p->mutex);
-    }
+    p->writers_writing--;
+    pthread_cond_broadcast(&p->lock_free);
+    pthread_mutex_unlock(&p->mutex);
 #endif
-    return 0;
 }
 
 /*

diff --git a/src/ppmutex.h b/src/ppmutex.h
index a3c6146..f47dfd9 100644 (file)
--- a/src/ppmutex.h
+++ b/src/ppmutex.h
@@ -43,13 +43,14 @@ typedef struct {
 #endif
 } Pazpar2_lock_rdwr;
 
-int pazpar2_lock_rdwr_init(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_destroy(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_rlock(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_wlock(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_runlock(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_wunlock(Pazpar2_lock_rdwr *p);
-int pazpar2_lock_rdwr_upgrade(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_init(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_destroy(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_rlock(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_wlock(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_runlock(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_wunlock(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_upgrade(Pazpar2_lock_rdwr *p);
+void pazpar2_lock_rdwr_downgrade(Pazpar2_lock_rdwr *p);
 
 #endif
 

diff --git a/src/session.c b/src/session.c
index 4ce25c3..e923abf 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -152,7 +152,7 @@ void session_enter_ro(struct session *s, const char *caller)
 {
     assert(s);
     if (caller)
-        session_log(s, YLOG_DEBUG, "Session read lock by %s", caller);
+        session_log(s, YLOG_LOG, "Session read lock by %s", caller);
     pazpar2_lock_rdwr_rlock(&s->lock);
 }
 
@@ -160,15 +160,23 @@ void session_enter_rw(struct session *s, const char *caller)
 {
     assert(s);
     if (caller)
-        session_log(s, YLOG_DEBUG, "Session write lock by %s", caller);
+        session_log(s, YLOG_LOG, "Session write lock by %s", caller);
     pazpar2_lock_rdwr_wlock(&s->lock);
 }
 
+void session_upgrade(struct session *s, const char *caller)
+{
+    assert(s);
+    if (caller)
+        session_log(s, YLOG_LOG, "Session upgrade lock by %s", caller);
+    pazpar2_lock_rdwr_upgrade(&s->lock);
+}
+
 void session_leave_ro(struct session *s, const char *caller)
 {
     assert(s);
     if (caller)
-        session_log(s, YLOG_DEBUG, "Session read unlock by %s", caller);
+        session_log(s, YLOG_LOG, "Session read unlock by %s", caller);
     pazpar2_lock_rdwr_runlock(&s->lock);
 }
 
@@ -176,10 +184,18 @@ void session_leave_rw(struct session *s, const char *caller)
 {
     assert(s);
     if (caller)
-        session_log(s, YLOG_DEBUG, "Session write unlock by %s", caller);
+        session_log(s, YLOG_LOG, "Session write unlock by %s", caller);
     pazpar2_lock_rdwr_wunlock(&s->lock);
 }
 
+void session_downgrade(struct session *s, const char *caller)
+{
+    assert(s);
+    if (caller)
+        session_log(s, YLOG_LOG, "Session write unlock by %s", caller);
+    pazpar2_lock_rdwr_downgrade(&s->lock);
+}
+
 static void session_normalize_facet(struct session *s, const char *type,
                                     const char *value,
                                     WRBUF display_wrbuf,
@@ -648,7 +664,8 @@ static void session_clear_set(struct session *se, struct reclist_sortparms *sp)
     se->reclist = reclist_create(se->nmem);
 }
 
-static void session_sort_unlocked(struct session *se, struct reclist_sortparms *sp)
+static void session_sort_unlocked(struct session *se,
+                                  struct reclist_sortparms *sp)
 {
     struct reclist_sortparms *sr;
     struct client_list *l;
@@ -717,21 +734,22 @@ static void session_sort_unlocked(struct session *se, struct reclist_sortparms *
     }
 }
 
-void session_sort(struct session *se, struct reclist_sortparms *sp) {
-    //session_enter(se, "session_sort");
+void session_sort(struct session *se, struct reclist_sortparms *sp)
+{
+    session_enter_rw(se, "session_sort");
     session_sort_unlocked(se, sp);
-    //session_leave(se, "session_sort");
+    session_leave_rw(se, "session_sort");
 }
 
-
-enum pazpar2_error_code session_search(struct session *se,
-                                       const char *query,
-                                       const char *startrecs,
-                                       const char *maxrecs,
-                                       const char *filter,
-                                       const char *limit,
-                                       const char **addinfo,
-                                       struct reclist_sortparms *sp)
+static
+enum pazpar2_error_code session_search_unlocked(struct session *se,
+                                                const char *query,
+                                                const char *startrecs,
+                                                const char *maxrecs,
+                                                const char *filter,
+                                                const char *limit,
+                                                const char **addinfo,
+                                                struct reclist_sortparms *sp)
 {
     int live_channels = 0;
     int no_working = 0;
@@ -743,11 +761,8 @@ enum pazpar2_error_code session_search(struct session *se,
 
     *addinfo = 0;
 
-    session_enter_rw(se, "session_search");
-
-    if (se->settings_modified) {
+    if (se->settings_modified)
         session_remove_cached_clients(se);
-    }
     else
         session_reset_active_clients(se, 0);
 
@@ -758,23 +773,18 @@ enum pazpar2_error_code session_search(struct session *se,
 
     live_channels = select_targets(se, filter);
     if (!live_channels)
-    {
-        session_leave_ro(se, "session_search");
         return PAZPAR2_NO_TARGETS;
-    }
 
     facet_limits_destroy(se->facet_limits);
     se->facet_limits = facet_limits_create(limit);
     if (!se->facet_limits)
     {
         *addinfo = "limit";
-        session_leave_ro(se, "session_search");
         return PAZPAR2_MALFORMED_PARAMETER_VALUE;
     }
 
     l0 = se->clients_active;
     se->clients_active = 0;
-    session_leave_ro(se, "session_search");
 
     for (l = l0; l; l = l->next)
     {
@@ -816,10 +826,26 @@ enum pazpar2_error_code session_search(struct session *se,
         else
             return PAZPAR2_NO_TARGETS;
     }
-    session_log(se, YLOG_LOG, "session_start_search done");
     return PAZPAR2_NO_ERROR;
 }
 
+enum pazpar2_error_code session_search(struct session *se,
+                                       const char *query,
+                                       const char *startrecs,
+                                       const char *maxrecs,
+                                       const char *filter,
+                                       const char *limit,
+                                       const char **addinfo,
+                                       struct reclist_sortparms *sp)
+{
+    enum pazpar2_error_code c;
+    session_enter_rw(se, "session_search");
+    c = session_search_unlocked(se, query, startrecs, maxrecs, filter,
+                                limit, addinfo, sp);
+    session_leave_rw(se, "session_search");
+    return c;
+}
+
 // Creates a new session_database object for a database
 static void session_init_databases_fun(void *context, struct database *db)
 {