[niepce] Rewrote the worker threads to be more thread safe. This should fix the concurrency issues found on s



commit c7d976069a3c713c358f59f42a16d59ed2eff056
Author: Hub Figuiere <hub figuiere net>
Date:   Wed Nov 9 23:57:17 2011 -0800

    Rewrote the worker threads to be more thread safe. This should
    fix the concurrency issues found on startup.
    Catch exception if one can't load the video thumbnail.

 src/engine/library/thumbnailcache.cpp |    9 ++-
 src/fwk/utils/thread.cpp              |   56 ++++++------
 src/fwk/utils/thread.hpp              |   43 ++++++----
 src/fwk/utils/worker.hpp              |  160 ++++++++++++++++++--------------
 4 files changed, 150 insertions(+), 118 deletions(-)
---
diff --git a/src/engine/library/thumbnailcache.cpp b/src/engine/library/thumbnailcache.cpp
index 7143fae..13dd665 100644
--- a/src/engine/library/thumbnailcache.cpp
+++ b/src/engine/library/thumbnailcache.cpp
@@ -91,8 +91,13 @@ Glib::RefPtr<Gdk::Pixbuf> getThumbnail(const LibFile::Ptr & f, int w, int h, con
     }
     // TODO: what about videos?
     else if(mime_type.isMovie()) {
-        if(fwk::thumbnail_movie(filename, w, h, cached)) {
-            pix = Gdk::Pixbuf::create_from_file(cached, w, h, true);
+        try {
+            if(fwk::thumbnail_movie(filename, w, h, cached)) {
+                pix = Gdk::Pixbuf::create_from_file(cached, w, h, true);
+            }
+        }
+        catch(const Glib::Error & e) {
+            ERR_OUT("exception %s", e.what().c_str());
         }
     }
     else if(!mime_type.isImage()) {
diff --git a/src/fwk/utils/thread.cpp b/src/fwk/utils/thread.cpp
index 47fbc77..16e1837 100644
--- a/src/fwk/utils/thread.cpp
+++ b/src/fwk/utils/thread.cpp
@@ -23,37 +23,33 @@
 namespace fwk {
 
 
-	Thread::Thread()
-		: m_terminated(true),
-		  m_thrd(NULL)
-	{
-	}
-
-
-	Thread::~Thread()
-	{
-		terminate();
-	}
-
-
-
-#if 0
-	void Thread::schedule(const Op::Ptr & _op)
-	{
-		OpQueue::mutex_t::scoped_lock lock(m_ops.mutex(), true);
-		bool was_empty = m_ops.isEmpty();
-		m_ops.add(_op);
-		if(was_empty) {
-			start();
-		}
-	}
-#endif
-
-	void Thread::start()
-	{
-		m_thrd = Glib::Thread::create(sigc::mem_fun(*this, &Thread::main), true);
+Thread::Thread()
+    : m_terminated(true),
+      m_thrd(NULL)
+{
+}
+
+
+Thread::~Thread()
+{
+    terminate();
+}
+
+
+void Thread::start()
+{
+    m_thrd = Glib::Thread::create(sigc::mem_fun(*this, &Thread::main), true);
 // TODO add this thread to a manager for task management.
 //		thrd->join();
-	}
+}
 
 }
+/*
+  Local Variables:
+  mode:c++
+  c-file-style:"stroustrup"
+  c-file-offsets:((innamespace . 0))
+  indent-tabs-mode:nil
+  fill-column:80
+  End:
+*/
diff --git a/src/fwk/utils/thread.hpp b/src/fwk/utils/thread.hpp
index d98bb72..0134dac 100644
--- a/src/fwk/utils/thread.hpp
+++ b/src/fwk/utils/thread.hpp
@@ -29,25 +29,36 @@
 
 namespace fwk {
 
-	/** thread */
-	class Thread
-	{
-	public:
-		/** create the worker for the library whose dir is specified */
-		Thread();
-		virtual ~Thread();
-
-		void terminate()
-			{ m_terminated = true; }
-	protected:
-		void start();
-		virtual void main() = 0;
-		volatile bool m_terminated;
-	private:
+/** thread */
+class Thread
+{
+public:
+    /** create the worker for the library whose dir is specified */
+    Thread();
+    virtual ~Thread();
+    
+    void terminate()
+        { m_terminated = true; }
+protected:
+    void start();
+    virtual void main() = 0;
+    volatile bool m_terminated;
+    Glib::Thread * thread() const
+        { return m_thrd; }
+private:
     Glib::Thread *       m_thrd;
-	};
+};
 
 }
 
 
 #endif
+/*
+  Local Variables:
+  mode:c++
+  c-file-style:"stroustrup"
+  c-file-offsets:((innamespace . 0))
+  indent-tabs-mode:nil
+  fill-column:80
+  End:
+*/
diff --git a/src/fwk/utils/worker.hpp b/src/fwk/utils/worker.hpp
index 405120e..da46f0d 100644
--- a/src/fwk/utils/worker.hpp
+++ b/src/fwk/utils/worker.hpp
@@ -29,83 +29,103 @@
 
 namespace fwk {
 
-	/** worker thread for the library */
-	template <class T>
-	class Worker
-		: public Thread
-	{
-	public:
-		Worker();
-		virtual ~Worker();
-		typedef MtQueue<T> queue_t;
-
+/** worker thread for the library */
+template <class T>
+class Worker
+    : public Thread
+{
+public:
+    Worker();
+    virtual ~Worker();
+    typedef MtQueue<T> queue_t;
+    
 #ifdef BOOST_AUTO_TEST_MAIN
-		queue_t & _tasks() 
-			{ return m_tasks; }
+    queue_t & _tasks() 
+        { return m_tasks; }
 #endif
-		void schedule(const T & );
-		void clear();
-	protected:
-		virtual void main();
-
-		queue_t      m_tasks;
-	private:
-		virtual void execute(const T & _op) = 0;
-	};
-
-	template <class T>
-	Worker<T>::Worker()
-		: Thread()
-	{
-	}
+    void schedule(const T & );
+    void clear();
+protected:
+    virtual void main();
+    
+    queue_t      m_tasks;
+private:
+    virtual void execute(const T & _op) = 0;
+    Glib::Mutex m_q_mutex;
+    Glib::Cond m_wait_cond;
+};
+
+template <class T>
+Worker<T>::Worker()
+    : Thread()
+{
+    start();
+}
 
-	template <class T>
-	Worker<T>::~Worker()
-	{
-		typename queue_t::mutex_t::Lock lock(m_tasks.mutex());
-		m_tasks.clear();
-	}
+template <class T>
+Worker<T>::~Worker()
+{
+    m_tasks.clear();
+    {
+        Glib::Mutex::Lock lock(m_q_mutex);
+        m_terminated = true;
+        m_wait_cond.broadcast();
+    }
+    thread()->join();
+}
 
-	/** this is the main loop of the libray worker */
-	template <class T>
-	void Worker<T>::main()
-	{
-		m_terminated = false;
-		
-		do 
-		{
-			T op;
-			{
-				// make sure we terminate the thread before we unlock
-				// the task queue.
-				typename queue_t::mutex_t::Lock lock(m_tasks.mutex());
-				if(m_tasks.empty()) {
-					m_terminated = true;
-					break;
-				}
-				op = m_tasks.pop();
-			}
-			execute(op);
-		} while(!m_terminated);
-	}
+/** this is the main loop of the libray worker */
+template <class T>
+void Worker<T>::main()
+{
+    m_terminated = false;
+    
+    do {
+        T op;
+        {
+            // make sure we terminate the thread before we unlock
+            // the task queue.
+            Glib::Mutex::Lock lock(m_q_mutex);
+            if(!m_tasks.empty()) {
+                op = m_tasks.pop();
+            }
+        }
+        // depending on T this might blow
+        if(op) {
+            execute(op);
+        }
+
+        Glib::Mutex::Lock lock(m_q_mutex);
+        if(m_tasks.empty()) {
+            m_wait_cond.wait(m_q_mutex);
+        }
+    } while(!m_terminated);
+}
 
-	template <class T>
-	void Worker<T>::schedule(const T & _op)
-	{
-		typename queue_t::mutex_t::Lock lock(m_tasks.mutex());
-		bool was_empty = m_tasks.empty();
-		m_tasks.add(_op);
-		if(was_empty) {
-			start();
-		}
-	}
+template <class T>
+void Worker<T>::schedule(const T & _op)
+{
+    Glib::Mutex::Lock lock(m_q_mutex);
+    m_tasks.add(_op);
+    m_wait_cond.broadcast();
+}
 
-	template <class T>
-	void Worker<T>::clear()
-	{
-		m_tasks.clear();
-	}
+template <class T>
+void Worker<T>::clear()
+{
+    Glib::Mutex::Lock lock(m_q_mutex);
+    m_tasks.clear();
+}
 
 }
 
 #endif
+/*
+  Local Variables:
+  mode:c++
+  c-file-style:"stroustrup"
+  c-file-offsets:((innamespace . 0))
+  indent-tabs-mode:nil
+  fill-column:80
+  End:
+*/



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]