consider advice::Index exception safety

This commit is contained in:
Fischlurch 2010-06-04 18:39:39 +02:00
parent 7895ce5f49
commit 5a615ee4f8
3 changed files with 27 additions and 7 deletions

View file

@ -163,6 +163,7 @@ namespace advice {
* Functor object for matching against another Binding.
* Contains precompiled information necessary for
* determining a match.
* @note Matcher is POD, copyable, no-throw
*/
class Matcher
{

View file

@ -140,6 +140,12 @@ namespace advice {
* explicitly relies on that definition of equality.
* @note the diagnostic API is mainly intended for unit testing
* and \em not implemented with focus on performance.
*
* \par Exception safety
* Adding new registrations might throw error::Fatal or bad_alloc.
* The addition in this case has no effect and the index remains valid.
* The other mutating operations are NO_THROW, given that Binding::Matcher
* is a POD and std::vector fulfils the guarantee for POD content elements.
*/
template<class POA>
class Index
@ -156,6 +162,8 @@ namespace advice {
: pair<Binding::Matcher, POA*> (getMatcher(elm), &elm)
{ }
// using default-copy, thus assuming copy is NO_THROW
friend bool
operator== (Entry const& a, Entry const& b)
{
@ -194,7 +202,12 @@ namespace advice {
append (POA& elm)
{
REQUIRE (!contains (elm), "Duplicate entry");
elms_.push_back (Entry(elm));
try { elms_.push_back (Entry(elm)); }
catch(std::bad_alloc&)
{
throw error::Fatal("AdviceSystem failure due to exhausted memory");
}
}
void
@ -337,7 +350,7 @@ namespace advice {
addRequest (POA& entry)
{
HashVal key (hash_value(entry));
requestEntries_[key].append (entry);
requestEntries_[key].append (entry); // might throw
provisionEntries_[key].publish_latest_solution (entry);
}
@ -353,8 +366,8 @@ namespace advice {
HashVal nKey (hash_value(entry));
if (oKey != nKey)
{
requestEntries_[nKey].append (entry); // might throw
requestEntries_[oKey].remove (entry);
requestEntries_[nKey].append (entry);
}
else
{ // rewrite Entry to include the new binding
@ -375,7 +388,7 @@ namespace advice {
addProvision (POA& entry)
{
HashVal key (hash_value(entry));
provisionEntries_[key].append (entry);
provisionEntries_[key].append (entry); // might throw
requestEntries_[key].publish_all_solutions (entry);
}
@ -386,8 +399,8 @@ namespace advice {
HashVal nKey (hash_value(newEntry));
if (oKey != nKey)
{
provisionEntries_[nKey].append (newEntry); // might throw, in which case it has no effect
provisionEntries_[oKey].remove (oldRef);
provisionEntries_[nKey].append (newEntry);
requestEntries_[nKey].publish_all_solutions (newEntry);
requestEntries_[oKey].retract_all_solutions (oldRef, provisionEntries_[oKey]);
}
@ -402,7 +415,7 @@ namespace advice {
removeProvision (POA const& refEntry)
{
HashVal key (hash_value(refEntry));
provisionEntries_[key].remove (refEntry);
provisionEntries_[key].remove (refEntry); // NO_THROW
requestEntries_[key].retract_all_solutions (refEntry, provisionEntries_[key]);
}

View file

@ -540,7 +540,7 @@ In a more elaborate scheme, the advised entity could provide a signal to be invo
&amp;rarr; AdviceImplementation
</pre>
</div>
<div title="AdviceImplementation" modifier="Ichthyostega" modified="201006020227" created="201004100056" tags="impl draft img" changecount="56">
<div title="AdviceImplementation" modifier="Ichthyostega" modified="201006041639" created="201004100056" tags="impl draft img" changecount="57">
<pre>[&lt;img[Advice solution|uml/fig141573.png]]
@ -590,6 +590,12 @@ Basically, the behaviour when requesting non-existing advice may be configured b
* on the other hand, using some separate kind of singleton bundle just for these default advice data (e.g. a templated version of //Meyer's Singleton...//), the fallback solution can be settled independent from the ~AdviceSystem, right in the {{{advice.hpp}}} and using static memory, but the downside is now the fallback solution might be destroyed prior to shutdown of the ~AdviceSystem, as it lives in another compilation unit.
Thus the second approach looks favourable, but we should //note the fact that it is hard to secure this possible access to an already destroyed solution,// unless we decline using the advice feature after the end of {{{main()}}}. Such a policy seems to be reasonable anyway, as the current implementation also has difficulties to prevent accessing an already destroyed {{{advice::Provision}}}, being incorporated in the ~AdviceSystem, but accessed through a direct pointer in the {{{advice::Request}}}.
!!!locking and exception safety
The advice system is (hopefully) written such as not to be corrupted in case an exception is thrown. Adding new requests, setting advice data on a provision and any binding change might fail due to exhausted memory. The advice system remains operational in this case, but the usual reaction would be //subsystem shutdown,// because the Advice facility typically is used in a very low-level manner, assuming it //just works.// As far as I can see, the other mutation operations can't throw.
The individual operations on the interface objects are //deliberately not thread-safe.// The general assumption is that {{{advice::Request}}} and {{{advice::Provision}}} will be used in a safe environment and not be accessed or modified concurrently. An notable exception to this rule is accessing Advice: as this just includes checking and dereferentiating a pointer, it might be done concurrently. But note, //the advice system does nothing to ensure visibility of the solution within a separate thread.// If this thread still has the old pointer value in his local cache, it won't pick up the new solution. In case the old solution got retracted, this even might cause access to already released objects. You have been warned. So it's probably a good idea to ensure a read barrier happens somewhere in the enclosing usage context prior to picking up a possibly changed advice solution concurrently.
{{red{TODO}}}: the underlying operations on the embedded global {{{advice::Index}}} obviously need to be protected by locking the whole index table on each mutation, which also ensures a memory barrier and thus propagates changed solutions.
!!!index datastructure
It is clear by now that the implementation datastrucutre has to serve as a kind of //reference count.// Within this datastructure, any constructed advice solution needs to be reflected somehow, to prevent us from discarding an advice provision still accessible. Allowing lock-free access to the advice solution (planned feature) adds an special twist, because in this case we can't even tell for sure if an overwritten old solution is actually gone (or if its still referred from some thread's cached memeory). This could be addressed with an transactional approach (which might be good anyway) &amp;mdash; but I tend to leave this special concern asside for now.