From 17fbc7d1cafce9c41e6397739fb5f11a5284dcbe Mon Sep 17 00:00:00 2001
From: ponchio <federico.ponchio@isti.cnr.it>
Date: Tue, 20 Dec 2011 18:25:13 +0000
Subject: [PATCH] hopefully fixed race condition updating priorities

---
 wrap/gcache/cache.h      |  3 ++-
 wrap/gcache/controller.h |  1 +
 wrap/gcache/provider.h   | 14 ++++----------
 wrap/gcache/token.h      | 15 +++++----------
 4 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/wrap/gcache/cache.h b/wrap/gcache/cache.h
index 8d26ea2c..1fdea02d 100644
--- a/wrap/gcache/cache.h
+++ b/wrap/gcache/cache.h
@@ -169,6 +169,7 @@ protected:
       mt::mutexlocker locker(&(this->heap_lock));
 
       //2 we have some element not in the upper caches (heap.size()  > 0
+      assert(this->heap.size());
       if(this->heap.size()) {
         Token &last = this->heap.min();
         int itemsize = size(&last);
@@ -228,7 +229,7 @@ protected:
       if(input->heap.size()) {                           //we need something in input to tranfer.
         Token &first = input->heap.max();
         if(first.count > Token::REMOVE &&
-            (!last || last->priority < first.priority)) { //if !last we already decided we want a transfer., otherwise check for a swap
+            (!last || first.priority > last->priority)) { //if !last we already decided we want a transfer., otherwise check for a swap
           insert = input->heap.popMax();                 //remove item from heap, while we transfer it.
         }
       }
diff --git a/wrap/gcache/controller.h b/wrap/gcache/controller.h
index 81241f84..960f9c54 100644
--- a/wrap/gcache/controller.h
+++ b/wrap/gcache/controller.h
@@ -64,6 +64,7 @@ class Controller {
   }
 
   ///ensure that added tokens are processed and existing ones have their priority updated.
+  ///potential bug! update is done on the heaps, if something is in transit...
   void updatePriorities() {
 
     if(tokens.size()) {
diff --git a/wrap/gcache/provider.h b/wrap/gcache/provider.h
index 9da0d29d..f6d0cce8 100644
--- a/wrap/gcache/provider.h
+++ b/wrap/gcache/provider.h
@@ -28,19 +28,18 @@ class Provider: public mt::thread {
   bool heap_dirty;
   ///lock this before manipulating heap.
   mt::mutex heap_lock;
-  ///used to sincronize priorities update
-  mt::mutex priority_lock;
   ///signals (to next cache!) priorities have changed or something is available
   QDoor check_queue;
 
   Provider(): max_tokens(-1), heap_dirty(false) {}
   virtual ~Provider() {}
 
-  /// [should be protected, do not use]
+  /// [should be protected, do not use] called in controller thread!
   void pushPriorities() {
-    mt::mutexlocker locker(&priority_lock);
+    mt::mutexlocker locker(&heap_lock);
     for(int i = 0; i < heap.size(); i++)
       heap[i].pushPriority();
+
     heap_dirty = true;
     check_queue.open();
   }
@@ -48,13 +47,8 @@ class Provider: public mt::thread {
   void rebuild() {
     if(!this->heap_dirty) return;
 
-    {
-      mt::mutexlocker locker(&priority_lock);
-      for(int i = 0; i < this->heap.size(); i++)
-        this->heap[i].pullPriority();
-      this->heap_dirty = false;
-    }
     this->heap.rebuild();
+    this->heap_dirty = false;
 
     //remove OUTSIDE tokens from bottom of heap
     if(max_tokens != -1) {
diff --git a/wrap/gcache/token.h b/wrap/gcache/token.h
index 89951d9b..deba63c3 100644
--- a/wrap/gcache/token.h
+++ b/wrap/gcache/token.h
@@ -28,12 +28,10 @@ class Token {
   enum Status { LOCKED = 1, READY = 0, CACHE = -1, REMOVE = -2, OUTSIDE = -3 };
   ///Do not access these members directly. Will be moved to private shortly.
   ///used by various cache threads to sort objects [do not use, should be private]
-  Priority priority; 
+  Priority priority;
   ///set in the main thread   [do not use, should be private]
-  Priority new_priority;     
+  Priority new_priority;
   ///swap space used in updatePriorities [do not use, should be private]
-  Priority tmp_priority;     
-  ///reference count of locked items [do not use, should be private]
   mt::atomicInt count;
 
  public:
@@ -43,6 +41,7 @@ class Token {
   void setPriority(const Priority &p) {
     new_priority = p;
   }
+  //set and get are safe to call in the controller thread.
   Priority getPriority() {
     return new_priority;
   }
@@ -53,7 +52,7 @@ class Token {
     return false;
   }
   ///assumes it was locked first and 1 unlock for each lock.
-  bool unlock() {  
+  bool unlock() {
     return count.deref();
   }
 
@@ -69,11 +68,7 @@ class Token {
 
   ///copy priority to swap space [do not use, should be private]
   void pushPriority() {
-    tmp_priority = new_priority;
-  }
-  ///copy priority from swap space [do not use, should be private]
-  void pullPriority() {
-    priority = tmp_priority;
+    priority = new_priority;
   }
 
   bool operator<(const Token &a) const {