From 01e2478b5101bf476bc9f972e1727543caefd4d8 Mon Sep 17 00:00:00 2001 From: Chris Tallon Date: Tue, 24 Jan 2017 21:01:12 +0000 Subject: [PATCH] Fix TVMEDIA channel VDR_PacketReceiver objects being leaked --- eventdispatcher.cc | 37 +++++++++++++++++------------ eventdispatcher.h | 7 ++++-- vdr.cc | 58 +++++++++++++++++++++++++++------------------- vdr.h | 2 +- 4 files changed, 62 insertions(+), 42 deletions(-) mode change 100755 => 100644 vdr.cc diff --git a/eventdispatcher.cc b/eventdispatcher.cc index dd1692b..fae5b10 100644 --- a/eventdispatcher.cc +++ b/eventdispatcher.cc @@ -61,24 +61,24 @@ bool EventDispatcher::edFindAndCall(void* userTag) edr->callinprogress = true; edUnlock(); - bool edr_delete=false; - bool edrType = edr->call(userTag,edr_delete); + bool r_deregisterEDR = false; + bool r_wakeThread = false; + bool r_deleteEDR = false; + edr->call(userTag, r_deregisterEDR, r_wakeThread, r_deleteEDR); edLock(); edr->callinprogress = false; - if (edrType == false) // it's a multicall + + // if it's a stream packet (r_deregisterEDR == false) + // and edr->nomorecalls == true + // then something has called unregister for this EDR while we were out on the call + // set r_wakeThread to ensure that we signal the edr->cond to wake up that thread in edUnregister below + if ((r_deregisterEDR == false) && (edr->nomorecalls)) { - if (edr->nomorecalls) // External has called unRegister - probably the receiver - { - // wake up the thread waiting in unregister - #ifndef WIN32 - pthread_cond_signal(&edr->cond); - #else - SetEvent(edr->cond); - #endif - } + r_wakeThread = true; } - else // It's a single call. The receiver should be removed from the list. There will be a thread to wake up + + if (r_deregisterEDR) { for(i = receivers.begin(); i != receivers.end(); i++) { @@ -92,13 +92,20 @@ bool EventDispatcher::edFindAndCall(void* userTag) if (i == receivers.end()) abort(); // should never happen // but it can happen under windows ... how?? #endif - + } + + if (r_wakeThread) + { #ifndef WIN32 pthread_cond_signal(&edr->cond); #else SetEvent(edr->cond); #endif - if (edr_delete) delete edr; + } + + if (r_deleteEDR) + { + delete edr; } edUnlock(); diff --git a/eventdispatcher.h b/eventdispatcher.h index 500ec59..3471754 100644 --- a/eventdispatcher.h +++ b/eventdispatcher.h @@ -44,8 +44,11 @@ class EDReceiver //(implementation in eventdispatcher.cc) virtual ~EDReceiver(); protected: - virtual bool call(void* userTag, bool & deleteme)=0; // Implementor must override this and do the actual call - // return true to have EventDispatcher remove receiver from list after call + virtual void call(void* userTag, bool& r_deregisterEDR, bool& r_wakeThread, bool& deleteEDR)=0; // Implementor must override this and do the actual call + // In the call set the following bools: + // r_deregisterEDR: true = remove EDReceiver from list after call + // r_wakeThread: true = wake up a RequestResponse thread which is waiting for a VDR response + // r_deleteEDR: true = delete the EDReceiver object once the call has finished bool nomorecalls; bool callinprogress; diff --git a/vdr.cc b/vdr.cc old mode 100755 new mode 100644 index db0dc9d..28724a2 --- a/vdr.cc +++ b/vdr.cc @@ -522,11 +522,12 @@ bool VDR::ed_cb_find(EDReceiver* edr, void* userTag) else if (packetChannel == CHANNEL_STREAM) { if (vdrpr->streamID == vresp->getStreamID()) return true; - } else if (packetChannel == CHANNEL_TVMEDIA) - { //We want them all - return true; } - + else if (packetChannel == CHANNEL_TVMEDIA) + { + if (vdrpr->streamID == vresp->getStreamID()) return true; + } + return false; } @@ -597,7 +598,7 @@ bool VDR::sendKA(ULONG timeStamp) // Here VDR takes a break for the VDR_PacketReceiver helper class -bool VDR_PacketReceiver::call(void* userTag, bool & deleteme) +void VDR_PacketReceiver::call(void* userTag, bool& r_deregisterEDR, bool& r_wakeThread, bool& r_deleteEDR) { if (receiverChannel == VDR::CHANNEL_REQUEST_RESPONSE) { @@ -605,33 +606,42 @@ bool VDR_PacketReceiver::call(void* userTag, bool & deleteme) // VDR::RequestResponse will be blocking waiting for this to happen. // That function has a pointer to this object and can read save_vresp. save_vresp = (VDR_ResponsePacket*)userTag; - deleteme=false; - return true; // Signals ED to remove edr from receivers and wake up edr thread + + r_deregisterEDR = true; + r_wakeThread = true; + r_deleteEDR = false; } - - if (receiverChannel == VDR::CHANNEL_STREAM) + else if (receiverChannel == VDR::CHANNEL_STREAM) { - // It's a stream packet. + // It's a stream packet. Pass off the stream data to streamReceiver, + // delete the vresp. Keep this PacketReceiver for the next stream packet. VDR_ResponsePacket* vresp = (VDR_ResponsePacket*)userTag; streamReceiver->streamReceive(vresp->getFlag(), vresp->getUserData(), vresp->getUserDataLength()); delete vresp; - deleteme=false; - return false; + + r_deregisterEDR = false; + r_wakeThread = false; + r_deleteEDR = false; } - if (receiverChannel == VDR::CHANNEL_TVMEDIA) + else if (receiverChannel == VDR::CHANNEL_TVMEDIA) { - // It's TVMedia - VDR_ResponsePacket* vresp = (VDR_ResponsePacket*)userTag; - Log::getInstance()->log("VDR", Log::DEBUG, "TVMedia Pictures arrived VDR %x",vresp->getStreamID()); - OsdVector *osd=dynamic_cast(Osd::getInstance()); - if (osd) osd->getPictReader()->receivePicture(vresp); - else delete vresp; //nonsense - deleteme=false; - return true; - } + // It's TVMedia + // Pass off the vresp object to OSDVector + // This used to return true which would signal the cond (wake the thread) + // but am going to try setting this to false because I don't know that there is a thread to signal + // delete the EDR. It's made once per media requested and wasn't owned/deleted by anything before - - abort(); // unknown receiverChannel, should not happen + VDR_ResponsePacket* vresp = (VDR_ResponsePacket*)userTag; + Log::getInstance()->log("VDR", Log::DEBUG, "TVMedia Pictures arrived VDR %x", vresp->getStreamID()); + OsdVector *osd=dynamic_cast(Osd::getInstance()); + if (osd) osd->getPictReader()->receivePicture(vresp); + // else delete vresp; //nonsense // only rpi does CHANNEL_TVMEDIA, rpi has osdvector. therefore, can't get here. + + r_deregisterEDR = true; + r_wakeThread = false; + r_deleteEDR = true; + } + else abort(); // unknown receiverChannel, should not happen } ///////////////////////////////////////////////////////////////////////////// diff --git a/vdr.h b/vdr.h index 18aa348..6a7c072 100644 --- a/vdr.h +++ b/vdr.h @@ -101,7 +101,7 @@ class StreamReceiver class VDR_PacketReceiver : public EDReceiver // implementation in vdr.cc { public: - virtual bool call(void* userTag, bool & deleteme); + virtual void call(void* userTag, bool& r_deregisterEDR, bool& r_wakeThread, bool& r_deleteEDR); friend class VDR; protected: -- 2.39.2