From ca7aae920298cd6c36335037f3bef27775ff82a3 Mon Sep 17 00:00:00 2001
From: Chris Tallon <chris@vomp.tv>
Date: Mon, 2 Jun 2008 21:53:57 +0000
Subject: [PATCH] Fix timercall/boxstack->update thread clash with
 boxstack->remove

---
 boxstack.cc     | 32 ++++++++++++++++++++++++++++----
 boxx.h          |  9 +++++++++
 vaudioplayer.cc | 15 ++++++++++++---
 vaudioplayer.h  |  1 +
 vepg.cc         |  6 +++++-
 vepg.h          |  1 +
 vpicture.cc     | 11 ++++++++---
 vpicture.h      |  1 +
 vradiorec.cc    |  8 ++++++--
 vradiorec.h     |  1 +
 vtimerlist.cc   |  6 +++++-
 vtimerlist.h    |  1 +
 vvideolivetv.cc |  5 ++++-
 vvideolivetv.h  |  1 +
 vvideorec.cc    |  9 ++++++---
 vvideorec.h     |  1 +
 vwelcome.cc     |  6 +++++-
 vwelcome.h      |  2 +-
 18 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/boxstack.cc b/boxstack.cc
index e4c620f..896b42c 100644
--- a/boxstack.cc
+++ b/boxstack.cc
@@ -78,6 +78,7 @@ int BoxStack::add(Boxx* v)
 #else
   WaitForSingleObject(boxLock, INFINITE);
 #endif
+  Log::getInstance()->log("BoxStack", Log::DEBUG, "Locked for add");  
   
   if (numBoxes == 16) return 0;
   boxes[numBoxes++] = v;
@@ -88,6 +89,8 @@ int BoxStack::add(Boxx* v)
   ReleaseMutex(boxLock);
 #endif
 
+  Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocked for add");
+  
   return 1;
 }
 
@@ -96,14 +99,26 @@ int BoxStack::add(Boxx* v)
 int BoxStack::remove(Boxx* toDelete)
 {
   if (!initted) return 0;
-  
+
+  toDelete->preDelete();
+
 #ifndef WIN32
   pthread_mutex_lock(&boxLock);
 #else
   WaitForSingleObject(boxLock, INFINITE);
 #endif
+  Log::getInstance()->log("BoxStack", Log::DEBUG, "Locked for remove");  
   
-  if (numBoxes == 0) return 0;
+  if (numBoxes == 0)
+  {           // FIXME
+            #ifndef WIN32
+            pthread_mutex_unlock(&boxLock);
+          #else
+            ReleaseMutex(boxLock);
+          #endif
+            Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocked for remove");  
+              return 0;
+  }
 
 //  Log::getInstance()->log("BoxStack", Log::DEBUG, "entering remove, numBoxes=%i", numBoxes);
 
@@ -126,7 +141,15 @@ int BoxStack::remove(Boxx* toDelete)
     if (i == -1)
     {
       // not a Box we have!
-      return 0;
+        {           // FIXME
+            #ifndef WIN32
+            pthread_mutex_unlock(&boxLock);
+          #else
+            ReleaseMutex(boxLock);
+          #endif
+            Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocked for remove");  
+              return 0;
+        }
     }
   }
 
@@ -155,6 +178,7 @@ int BoxStack::remove(Boxx* toDelete)
 #else
   ReleaseMutex(boxLock);
 #endif
+  Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocked for remove");  
 
   return 1;
 }
@@ -227,12 +251,12 @@ void BoxStack::update(Boxx* toUpdate, Region* regionToUpdate)
     rl.pop_front();
   }
   
-  Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocking");
 #ifndef WIN32
   pthread_mutex_unlock(&boxLock);
 #else
   ReleaseMutex(boxLock);
 #endif  
+  Log::getInstance()->log("BoxStack", Log::DEBUG, "Unlocked for update");
 }
 
 void BoxStack::repaintRevealed(int x, Region r)
diff --git a/boxx.h b/boxx.h
index 11a8366..f9da2e1 100644
--- a/boxx.h
+++ b/boxx.h
@@ -56,12 +56,21 @@ class Boxx
     // The following are supposed to be abstract functions
     // However, it is useful to be able to make instances of Boxx
     // Therefore the following stubs are provided.
+    virtual void preDelete() {}
     virtual int handleCommand(int x) { return 0; }
     virtual void processMessage(Message* m) {}
     virtual bool mouseMove(int x, int y) { return false; }
     virtual bool mouseLBDOWN(int x, int y) { return false; }
     virtual void deactivateAllControls() {}
 
+    /* preDelete 
+    
+     I think it's functionally equivalent to e.g. delete timers in Boxx::preDelete
+     because the only place where a Boxx is deleted is in 
+     BoxStack::remove. There is now a call in BoxStack::remove to Boxx::preDelete
+     The reason for this is to stop timercalls calling BoxStack::update at the
+     same time BoxStack::remove is locked trying to delete the Boxx
+    */
 
     // Get functions
     int getScreenX();        // where is it on screen
diff --git a/vaudioplayer.cc b/vaudioplayer.cc
index b50feb3..882e8b7 100644
--- a/vaudioplayer.cc
+++ b/vaudioplayer.cc
@@ -66,6 +66,16 @@ AudioPlayer * VAudioplayer::getPlayer(bool createIfNeeded)
   return rt;
 }
 
+void VAudioplayer::preDelete()
+{
+  // Another note from Chris:
+  // I have moved these timer cancels here from the destructor, please see boxx.h
+
+  Timers::getInstance()->cancelTimer(this,1);
+  Timers::getInstance()->cancelTimer(this,2);
+  Timers::getInstance()->cancelTimer(this,3);
+}
+
 VAudioplayer::~VAudioplayer()
 {
   // Note from Chris:
@@ -78,9 +88,8 @@ VAudioplayer::~VAudioplayer()
   if (banner) BoxStack::getInstance()->remove(banner);
   if (fullname) delete fullname;
   if (filename) delete filename;
-  Timers::getInstance()->cancelTimer(this,1);
-  Timers::getInstance()->cancelTimer(this,2);
-  Timers::getInstance()->cancelTimer(this,3);
+
+
   //TODO leave this to medialist and stop only...
   if (getPlayer(false)) {
     AudioPlayer::getInstance(NULL,false)->shutdown();
diff --git a/vaudioplayer.h b/vaudioplayer.h
index 219ea76..e8746ad 100644
--- a/vaudioplayer.h
+++ b/vaudioplayer.h
@@ -47,6 +47,7 @@ class VAudioplayer : public TBBoxx, public TimerReceiver
 
     void processMessage(Message* m);
     int handleCommand(int command);
+    void preDelete();
     void draw();
     void timercall(int clientReference);
     //factory method
diff --git a/vepg.cc b/vepg.cc
index f6dfb55..3531d02 100644
--- a/vepg.cc
+++ b/vepg.cc
@@ -161,9 +161,13 @@ VEpg::VEpg(void* tparent, UINT tcurrentChannelIndex, ULONG streamType)
   updateEventList(); // get list of programmes
 }
 
-VEpg::~VEpg()
+void VEpg::preDelete()
 {
   Timers::getInstance()->cancelTimer(this, 1);
+}
+
+VEpg::~VEpg()
+{
 
   instance = NULL;
 
diff --git a/vepg.h b/vepg.h
index 59db20c..edef8a6 100644
--- a/vepg.h
+++ b/vepg.h
@@ -48,6 +48,7 @@ class VEpg : public Boxx, public TimerReceiver
     ~VEpg();
     static VEpg* getInstance();
 
+    void preDelete();
     int handleCommand(int command); // deal with commands (from remote control)
     void draw(); // draw epg view
     void processMessage(Message* m);
diff --git a/vpicture.cc b/vpicture.cc
index 7ad5f43..28320df 100644
--- a/vpicture.cc
+++ b/vpicture.cc
@@ -90,15 +90,20 @@ VPicture::VPicture(VMediaList *p)
   add(&jpeg);
 }
 
+void VPicture::preDelete()
+{
+  Timers::getInstance()->cancelTimer(this,1);
+  Timers::getInstance()->cancelTimer(this,2);
+  Timers::getInstance()->cancelTimer(this,3);
+}
+
 VPicture::~VPicture()
 {
   delete reader;
   if (banner) BoxStack::getInstance()->remove(banner);
   if (fullname) delete fullname;
   if (filename) delete filename;
-  Timers::getInstance()->cancelTimer(this,1);
-  Timers::getInstance()->cancelTimer(this,2);
-  Timers::getInstance()->cancelTimer(this,3);
+
   destroyInfo();
   
 }
diff --git a/vpicture.h b/vpicture.h
index 0f54c72..a55a6c0 100644
--- a/vpicture.h
+++ b/vpicture.h
@@ -46,6 +46,7 @@ class VPicture : public Boxx, public TimerReceiver
   public:
     ~VPicture();
 
+    void preDelete();
     void processMessage(Message* m);
     int handleCommand(int command);
     void draw();
diff --git a/vradiorec.cc b/vradiorec.cc
index 5c8737f..810a4f6 100644
--- a/vradiorec.cc
+++ b/vradiorec.cc
@@ -92,12 +92,16 @@ VRadioRec::VRadioRec(Recording* rec)
   barShowing = false;
 }
 
+void VRadioRec::preDelete()
+{
+  timers->cancelTimer(this, 1);
+  timers->cancelTimer(this, 2);
+}
+
 VRadioRec::~VRadioRec()
 {
   if (playing) stopPlay();
 
-  timers->cancelTimer(this, 1);
-  timers->cancelTimer(this, 2);
 
   // kill recInfo in case resumePoint has changed (likely)
   myRec->dropRecInfo();
diff --git a/vradiorec.h b/vradiorec.h
index b7cc103..73db089 100644
--- a/vradiorec.h
+++ b/vradiorec.h
@@ -42,6 +42,7 @@ class VRadioRec : public Boxx, public TimerReceiver
     VRadioRec(Recording* rec);
     ~VRadioRec();
     void draw();
+    void preDelete();
     int handleCommand(int command);
     void go();
 
diff --git a/vtimerlist.cc b/vtimerlist.cc
index 23d6cd7..d2b38ea 100644
--- a/vtimerlist.cc
+++ b/vtimerlist.cc
@@ -69,9 +69,13 @@ VTimerList::VTimerList()
   add(&sl);
 }
 
-VTimerList::~VTimerList()
+void VTimerList::preDelete()
 {
   Timers::getInstance()->cancelTimer(this, 1);
+}
+
+VTimerList::~VTimerList()
+{
   if (recTimerList)
   {
     for (UINT i = 0; i < recTimerList->size(); i++)
diff --git a/vtimerlist.h b/vtimerlist.h
index c3690a2..a7f6a10 100644
--- a/vtimerlist.h
+++ b/vtimerlist.h
@@ -38,6 +38,7 @@ class VTimerList : public TBBoxx, public TimerReceiver
   public:
     VTimerList();
     ~VTimerList();
+    void preDelete();
 
     int handleCommand(int command);
     void timercall(int clientReference);
diff --git a/vvideolivetv.cc b/vvideolivetv.cc
index 358b3cd..53599e5 100644
--- a/vvideolivetv.cc
+++ b/vvideolivetv.cc
@@ -235,10 +235,13 @@ VVideoLiveTV::VVideoLiveTV(ChannelList* tchanList, ULONG initialChannelNumber, V
   osdSummaryRegion = r1 + r2;
 }
 
-VVideoLiveTV::~VVideoLiveTV()
+void VVideoLiveTV::preDelete()
 {
   if (playing) stop();
+}
 
+VVideoLiveTV::~VVideoLiveTV()
+{
   delete player;
   video->setDefaultAspect();
   delData();
diff --git a/vvideolivetv.h b/vvideolivetv.h
index 54a55f6..d261d98 100644
--- a/vvideolivetv.h
+++ b/vvideolivetv.h
@@ -48,6 +48,7 @@ class VVideoLiveTV : public Boxx, public TimerReceiver
   public:
     VVideoLiveTV(ChannelList* chanList, ULONG initialChannelNumber, VChannelList* vchannelList);
     ~VVideoLiveTV();
+    void preDelete();
     int handleCommand(int command);
     void processMessage(Message* m);
 
diff --git a/vvideorec.cc b/vvideorec.cc
index 0475d98..913d1bc 100644
--- a/vvideorec.cc
+++ b/vvideorec.cc
@@ -125,6 +125,12 @@ VVideoRec::VVideoRec(Recording* rec)
   }
 }
 
+void VVideoRec::preDelete()
+{
+  timers->cancelTimer(this, 1);
+  timers->cancelTimer(this, 2);
+}
+
 VVideoRec::~VVideoRec()
 {
   Log::getInstance()->log("VVideoRec", Log::DEBUG, "Entering vvideorec destructor");
@@ -140,9 +146,6 @@ VVideoRec::~VVideoRec()
   if (playing) stopPlay();
   video->setDefaultAspect();
 
-  timers->cancelTimer(this, 1);
-  timers->cancelTimer(this, 2);
-
   // kill recInfo in case resumePoint has changed (likely)
   myRec->dropRecInfo();
   // FIXME - do this properly - save the resume point back to the server manually and update
diff --git a/vvideorec.h b/vvideorec.h
index a00f003..36f5231 100644
--- a/vvideorec.h
+++ b/vvideorec.h
@@ -46,6 +46,7 @@ class VVideoRec : public Boxx, public TimerReceiver
   public:
     VVideoRec(Recording* rec);
     ~VVideoRec();
+    void preDelete();
     int handleCommand(int command);
     void go(bool resume);
 
diff --git a/vwelcome.cc b/vwelcome.cc
index 6325309..6306fc6 100644
--- a/vwelcome.cc
+++ b/vwelcome.cc
@@ -77,11 +77,15 @@ VWelcome::VWelcome()
   add(&jpeg);
 }
 
-VWelcome::~VWelcome()
+void VWelcome::preDelete()
 {
   Timers::getInstance()->cancelTimer(this, 1);
 }
 
+VWelcome::~VWelcome()
+{
+}
+
 void VWelcome::draw()
 {
   TBBoxx::draw();
diff --git a/vwelcome.h b/vwelcome.h
index ced2fb5..90cf754 100644
--- a/vwelcome.h
+++ b/vwelcome.h
@@ -40,7 +40,7 @@ class VWelcome : public TBBoxx, public TimerReceiver
     ~VWelcome();
 
     void create();
-
+    void preDelete();
     int handleCommand(int command);
     void processMessage(Message* m);
     void draw();
-- 
2.39.5