diff --git a/src/lib/advice.hpp b/src/lib/advice.hpp index a91c27db0..c1477b05f 100644 --- a/src/lib/advice.hpp +++ b/src/lib/advice.hpp @@ -169,14 +169,15 @@ namespace advice { : public PointOfAdvice { protected: - void publishProvision (PointOfAdvice*); - void discardSolutions (); + const PointOfAdvice* publishProvision (PointOfAdvice*); + const PointOfAdvice* discardSolutions (); void publishRequestBindingChange(HashVal); void registerRequest(); void deregisterRequest(); void* getBuffer(size_t); + void releaseBuffer (const void*, size_t); public: explicit @@ -201,6 +202,7 @@ namespace advice { * Thus each Provision cares for "his" advice and just detaches when going away. Consequently, by default, advice provisions * aren't freed during the lifetime of the application. We'll see if this causes problems. * + * @todo currently leaking buffer storage for all the advice data which isn't explicitly retracted. */ template class Provision @@ -210,7 +212,11 @@ namespace advice { /* == policy definitions == */ ////TODO: extract into policy classes - void deregistrate() { /* NOP */ } + void + deregistrate() + { + TODO ("hand-over deallocation information to the AdviceSystem"); + } public: @@ -227,12 +233,15 @@ namespace advice { void setAdvice (AD const& pieceOfAdvice) { - publishProvision (storeCopy (pieceOfAdvice)); + maybe_deallocateOld ( + publishProvision( + storeCopy (pieceOfAdvice))); } void retractAdvice() { - discardSolutions (); + maybe_deallocateOld( + discardSolutions()); } void @@ -244,6 +253,7 @@ namespace advice { private: PointOfAdvice* storeCopy (AD const& advice_given); + void maybe_deallocateOld(const PointOfAdvice*); void maybe_rePublish (); }; @@ -283,10 +293,12 @@ namespace advice { friend class Provision; }; - - /** function to copy advice into an internal buffer, + + /* ==== memory management for Provision data ===== */ + + /** function to copy advice into an internal buffer. @return type erased pointer to the data holder created - @throw error::External on allocation problems, plus anything + @throw error::Fatal on allocation problems, plus anything the advice data may throw during copy construction. */ template inline PointOfAdvice* @@ -297,9 +309,26 @@ namespace advice { } + /** @internal assist the AdviceSystem with deallocating buffer storage. + * Problem is we need to know the exact size of the advice value holder, + * which information is available only here, in the fully typed context. + * @note the assumption is that \em any binding created will automatically + * contain a type guard, which ensures the existingEntry passed in here + * originally was allocated by #storeCopy within the same typed context. + */ + template + inline void + Provision::maybe_deallocateOld (const PointOfAdvice* existingEntry) + { + typedef ActiveProvision Holder; + if (existingEntry) + releaseBuffer (existingEntry, sizeof(Holder)); + } + + /** @internal in case we've already published this provision, - * we temporarily need a new provision entry, to allow the - * AdviceSystem implementation to rewrite the internal index + * we temporarily need a new provision entry, to allow the + * AdviceSystem implementation to rewrite the internal index */ template inline void @@ -309,7 +338,9 @@ namespace advice { AdviceProvision* solution = static_cast (getSolution (*this)); if (solution) // create copy of the data holder, using the new binding - publishProvision (storeCopy (solution->getAdvice())); + maybe_deallocateOld ( + publishProvision( + storeCopy (solution->getAdvice()))); } diff --git a/src/lib/advice/advice.cpp b/src/lib/advice/advice.cpp index 84c13de09..17504e900 100644 --- a/src/lib/advice/advice.cpp +++ b/src/lib/advice/advice.cpp @@ -72,24 +72,42 @@ namespace advice { We need to manage this internally, as the original advice::Provision may go out of scope, while the advice information as such remains valid. @note the special twist is the size of the buffer depending on the actual - advice type, which we need to erase for tracking all advice provisions - and advice requests through an generic index datastructure. + advice type, which type information we need to erase for tracking all + advice provisions and requests through an generic index datastructure. @todo rewrite to use Lumiera's block allocator / memory pool */ void* - AdviceLink::getBuffer(size_t) + AdviceLink::getBuffer(size_t siz) { - UNIMPLEMENTED ("raw allocation and de-allocation of advice holding buffer"); + try { return new char[siz]; } + + catch(std::bad_alloc&) + { + throw error::Fatal("Unable to store Advice due to memory exhaustion"); + } } + void + AdviceLink::releaseBuffer (const void* buff, size_t) + { + delete[] (char*)buff; + } + + + /** when the Provision actually sets advice data, this is copied * into an internal buffer within the AdviceSystem. We then use the * Index to remember the presence of this advice data and to detect * possible matches with existing advice::Request entries. * @param adviceData pointer to the copied data, * actually pointing to an ActiveProvision + * @return pointer to an superseded old provision entry, + * which the caller then needs to de-allocate. + * The caller is assumed to know the actual type + * and thus the size of the entry to deallocate. + * Returning \c NULL in case no old entry exists. */ - void + const PointOfAdvice* AdviceLink::publishProvision (PointOfAdvice* newProvision) { const PointOfAdvice* previousProvision (getSolution (*this)); @@ -102,7 +120,9 @@ namespace advice { aSys().modifyProvision (*previousProvision, *newProvision); else if (previousProvision && !newProvision) - aSys().removeProvision (*previousProvision); ////////////////////////////TODO: don't we need to release buffer storage here? + aSys().removeProvision (*previousProvision); + + return previousProvision; // to be deallocated by caller if non-NULL } @@ -110,14 +130,18 @@ namespace advice { * after removing the provision index entry * we also need to re-process any requests * which happen to match our binding... + * @return pointer to the existing provision entry, + * to be deallocated by the caller, which + * is assumed to know it's exact type. */ - void + const PointOfAdvice* AdviceLink::discardSolutions () { const PointOfAdvice* existingProvision (getSolution (*this)); setSolution (this, NULL ); if (existingProvision) - aSys().removeProvision (*existingProvision); ////////////////////////////TODO: don't we need to release buffer storage here? + aSys().removeProvision (*existingProvision); + return existingProvision; }