Redo API for notifiers.

There's now an notifier::Modifiable interface class that class
supposed to signal modifications are to be derived from. This takes
the place of the former MutableValue class and also unifies how Val
and IDs signal modifications.

src/ID.cc

@@ -66,7 +66,7 @@ void ID::SetVal(Val* v, bool arg_weak_ref)
 
 	val = v;
 	weak_ref = arg_weak_ref;
-	notifiers.Modified(this);
+	Modified();
 
 #ifdef DEBUG
 	UpdateValID();

src/ID.h

@@ -15,7 +15,7 @@ class Func;
 typedef enum { INIT_NONE, INIT_FULL, INIT_EXTRA, INIT_REMOVE, } init_class;
 typedef enum { SCOPE_FUNCTION, SCOPE_MODULE, SCOPE_GLOBAL } IDScope;
 
-class ID : public BroObj {
+class ID : public BroObj, public notifier::Modifiable {
 public:
 	ID(const char* name, IDScope arg_scope, bool arg_is_export);
 	~ID() override;

src/StateAccess.cc

@@ -4,110 +4,57 @@
 #include "NetVar.h"
 #include "DebugLogger.h"
 
-NotifierRegistry notifiers;
+notifier::Registry notifier::registry;
 
-NotifierRegistry::~NotifierRegistry()
+notifier::Registry::~Registry()
 	{
-	for ( auto i : ids )
+	for ( auto i : registrations )
-		Unref(i.first);
+		Unregister(i.first);
-
-	for ( auto i : vals )
-		Unref(i.first);
 	}
 
-void NotifierRegistry::Register(ID* id, NotifierRegistry::Notifier* notifier)
+void notifier::Registry::Register(Modifiable* m, notifier::Notifier* notifier)
 	{
-	DBG_LOG(DBG_NOTIFIERS, "registering ID %s for notifier %s",
+	DBG_LOG(DBG_NOTIFIERS, "registering modifiable %p for notifier %s",
-		id->Name(), notifier->Name());
+		m, notifier->Name());
 
-	Attr* attr = new Attr(ATTR_TRACKED);
+	registrations.insert({m, notifier});
-
+	++m->notifiers;
-	if ( id->Attrs() )
-		{
-		if ( ! id->Attrs()->FindAttr(ATTR_TRACKED) )
-			id->Attrs()->AddAttr(attr);
-		}
-	else
-		{
-		attr_list* a = new attr_list{attr};
-		id->SetAttrs(new Attributes(a, id->Type(), false));
 	}
 
-	Unref(attr);
+void notifier::Registry::Unregister(Modifiable* m, notifier::Notifier* notifier)
-
-	ids.insert({id, notifier});
-	Ref(id);
-	}
-
-void NotifierRegistry::Register(Val* val, NotifierRegistry::Notifier* notifier)
 	{
-	if ( ! val->IsMutableVal() )
+	DBG_LOG(DBG_NOTIFIERS, "unregistering modifiable %p from notifier %s",
-		return;
+		m, notifier->Name());
 
-	DBG_LOG(DBG_NOTIFIERS, "registering value %p for notifier %s",
+	auto x = registrations.equal_range(m);
-		val, notifier->Name());
-
-	vals.insert({val, notifier});
-	Ref(val);
-	}
-
-void NotifierRegistry::Unregister(ID* id, NotifierRegistry::Notifier* notifier)
-	{
-	DBG_LOG(DBG_NOTIFIERS, "unregistering ID %s for notifier %s",
-		id->Name(), notifier->Name());
-
-	auto x = ids.equal_range(id);
 	for ( auto i = x.first; i != x.second; i++ )
 		{
 		if ( i->second == notifier )
 			{
-			ids.erase(i);
+			registrations.erase(i);
-			Unref(id);
+			--i->first->notifiers;
-			break;
-			}
-		}
-
-	if ( ids.find(id) == ids.end() )
-		// Last registered notifier for this ID
-		id->Attrs()->RemoveAttr(ATTR_TRACKED);
-	}
-
-void NotifierRegistry::Unregister(Val* val, NotifierRegistry::Notifier* notifier)
-	{
-	DBG_LOG(DBG_NOTIFIERS, "unregistering Val %p for notifier %s",
-		val, notifier->Name());
-
-	auto x = vals.equal_range(val);
-	for ( auto i = x.first; i != x.second; i++ )
-		{
-		if ( i->second == notifier )
-			{
-			vals.erase(i);
-			Unref(val);
 			break;
 			}
 		}
 	}
 
-void NotifierRegistry::Modified(Val *val)
+void notifier::Registry::Unregister(Modifiable* m)
 	{
-	DBG_LOG(DBG_NOTIFIERS, "modification to tracked value %p", val);
+	auto x = registrations.equal_range(m);
-
-	auto x = vals.equal_range(val);
 	for ( auto i = x.first; i != x.second; i++ )
-		i->second->Modified(val);
+		Unregister(m, i->second);
 	}
 
-void NotifierRegistry::Modified(ID *id)
+void notifier::Registry::Modified(Modifiable* m)
 	{
-	DBG_LOG(DBG_NOTIFIERS, "modification to tracked ID %s", id->Name());
+	DBG_LOG(DBG_NOTIFIERS, "modification to modifiable %p", m);
 
-	auto x = ids.equal_range(id);
+	auto x = registrations.equal_range(m);
 	for ( auto i = x.first; i != x.second; i++ )
-		i->second->Modified(id);
+		i->second->Modified(m);
 	}
 
-const char* NotifierRegistry::Notifier::Name() const
+const char* notifier::Notifier::Name() const
 	{
 	return fmt("%p", this);
 	}

src/StateAccess.h

@@ -1,4 +1,14 @@
 // A class describing a state-modyfing access to a Value or an ID.
+//
+// TODO UPDATE: We provide a notifier framework to inform interested parties
+// of modifications to selected global IDs/Vals. To get notified about a
+// change, derive a class from Notifier and register the interesting
+// instances with the NotifierRegistry.
+//
+// Note: For containers (e.g., tables), notifications are only issued if the
+// container itself is modified, *not* for changes to the values contained
+// therein.
+
 
 #ifndef STATEACESSS_H
 #define STATEACESSS_H
@@ -7,63 +17,74 @@
 #include <unordered_map>
 #include <string>
 
-class Val;
+#include "util.h"
-class ID;
-class MutableVal;
-class HashKey;
-class ODesc;
-class TableVal;
 
-// We provide a notifier framework to inform interested parties of
+namespace notifier  {
-// modifications to selected global IDs/Vals. To get notified about a change,
-// derive a class from Notifier and register the interesting IDs/Vals with
-// the NotifierRegistry.
-//
-// Note: For containers (e.g., tables), notifications are only issued if the
-// container itself is modified, *not* for changes to the values contained
-// therein.
 
-class NotifierRegistry {
+class Modifiable;
+
+class Notifier {
 public:
-	class Notifier {
-	public:
 	virtual ~Notifier()	{ }
 
-		// Called when a change is being performed. Note that when
+	// Called afger a change has been performed.
-		// these methods are called, it is undefined whether the
+	virtual void Modified(Modifiable* m) = 0;
-		// change has already been done or is just going to be
-		// performed soon.
-		virtual void Modified(ID* id) = 0;
-		virtual void Modified(Val* val) = 0;
 	virtual const char* Name() const;	// for debugging
-	};
-
-	NotifierRegistry()	{ }
-	~NotifierRegistry();
-
-	// Register a new notifier to be informed when ID/Val changes. Note
-	// that the registry will store a reference to the target, keeping
-	// the instance alive for as long as it's registered.
-	void Register(ID* id, Notifier* notifier);
-	void Register(Val* val, Notifier* notifier);
-
-	// Cancel a notifier's tracking for this ID/Val, also releasing the
-	// referencee being held.
-	void Unregister(ID* id, Notifier* notifier);
-	void Unregister(Val* val, Notifier* notifier);
-
-	// Inform all registered notifiiers of a modification to a value/ID.
-	void Modified(ID *id);
-	void Modified(Val *val);
-
-private:
-	typedef std::unordered_multimap<Val*, Notifier*> ValMap;
-	typedef std::unordered_multimap<ID*, Notifier*> IDMap;
-
-	ValMap vals;
-	IDMap ids;
 };
 
-extern NotifierRegistry notifiers;
+// Singleton class.
+class Registry {
+public:
+	Registry()	{ }
+	~Registry();
+
+	// Register a new notifier to be informed when an instance changes.
+	void Register(Modifiable* m, Notifier* notifier);
+
+	// Cancel a notifier's tracking an instace.
+	void Unregister(Modifiable* m, Notifier* notifier);
+
+	// Cancel all notifiers registered for an instance.
+	void Unregister(Modifiable* m);
+
+private:
+	friend class Modifiable;
+
+	// Inform all registered notifiers of a modification to an instance.
+	void Modified(Modifiable* m);
+
+	typedef std::unordered_multimap<Modifiable*, Notifier*> ModifiableMap;
+	ModifiableMap registrations;
+};
+
+
+class Registry;
+extern Registry registry;
+
+// Base class for objects wanting to signal modifications to the registry.
+class Modifiable {
+protected:
+	friend class Registry;
+
+	Modifiable()	{}
+	~Modifiable()
+		{
+		if ( notifiers )
+			registry.Unregister(this);
+
+		};
+
+	void Modified()
+		{
+		if ( notifiers )
+			registry.Modified(this);
+		}
+
+	// Number of currently registered notifiers for this instance.
+	uint64 notifiers;
+};
+
+}
+
 
 #endif

src/Trigger.cc

@@ -33,7 +33,7 @@ TraversalCode TriggerTraversalCallback::PreExpr(const Expr* expr)
 			trigger->Register(e->Id());
 
 		Val* v = e->Id()->ID_Val();
-		if ( v && v->IsMutableVal() )
+		if ( v && v->Modifiable() )
 			trigger->Register(v);
 		break;
 		};
@@ -382,38 +382,30 @@ void Trigger::Timeout()
 void Trigger::Register(ID* id)
 	{
 	assert(! disabled);
-	notifiers.Register(id, this);
+	notifier::registry.Register(id, this);
 
 	Ref(id);
-	ids.insert(id);
+	objs.push_back(id);
 	}
 
 void Trigger::Register(Val* val)
 	{
+	if ( ! val->Modifiable() )
+		return;
+
 	assert(! disabled);
-	notifiers.Register(val, this);
+	notifier::registry.Register(val->Modifiable(), this);
 
 	Ref(val);
-	vals.insert(val);
+	objs.push_back(val);
 	}
 
 void Trigger::UnregisterAll()
 	{
-	loop_over_list(ids, i)
+	for ( auto o : objs )
-		{
+		Unref(o); // this will unregister with the registry as well
-		notifiers.Unregister(ids[i], this);
-		Unref(ids[i]);
-		}
 
-	ids.clear();
+	objs.clear();
-
-	loop_over_list(vals, j)
-		{
-		notifiers.Unregister(vals[j], this);
-		Unref(vals[j]);
-		}
-
-	vals.clear();
 	}
 
 void Trigger::Attach(Trigger *trigger)

src/Trigger.h

@@ -13,7 +13,7 @@
 class TriggerTimer;
 class TriggerTraversalCallback;
 
-class Trigger : public NotifierRegistry::Notifier, public BroObj {
+class Trigger : public notifier::Notifier, public BroObj {
 public:
 	// Don't access Trigger objects; they take care of themselves after
 	// instantiation.  Note that if the condition is already true, the
@@ -61,9 +61,7 @@ public:
 		{ d->Add("<trigger>"); }
 	// Overidden from Notifier.  We queue the trigger and evaluate it
 	// later to avoid race conditions.
-	void Modified(ID* id) override
+	void Modified(notifier::Modifiable* m) override
-		{ QueueTrigger(this); }
-	void Modified(Val* val) override
 		{ QueueTrigger(this); }
 
 	const char* Name() const override;
@@ -104,8 +102,7 @@ private:
 	bool delayed; // true if a function call is currently being delayed
 	bool disabled;
 
-	val_list vals;
+	std::vector<BroObj *> objs;
-	id_list ids;
 
 	typedef map<const CallExpr*, Val*> ValCache;
 	ValCache cache;

src/Val.cc

@@ -899,7 +899,7 @@ static void table_entry_val_delete_func(void* val)
 	delete tv;
 	}
 
-TableVal::TableVal(TableType* t, Attributes* a) : MutableVal(t)
+TableVal::TableVal(TableType* t, Attributes* a) : Val(t)
 	{
 	Init(t);
 	SetAttrs(a);
@@ -1982,7 +1982,7 @@ unsigned int TableVal::MemoryAllocation() const
 
 vector<RecordVal*> RecordVal::parse_time_records;
 
-RecordVal::RecordVal(RecordType* t, bool init_fields) : MutableVal(t)
+RecordVal::RecordVal(RecordType* t, bool init_fields) : Val(t)
 	{
 	origin = 0;
 	record_type = t;
@@ -2287,7 +2287,7 @@ Val* EnumVal::DoClone(CloneState* state)
 	return Ref();
 	}
 
-VectorVal::VectorVal(VectorType* t) : MutableVal(t)
+VectorVal::VectorVal(VectorType* t) : Val(t)
 	{
 	vector_type = t->Ref()->AsVectorType();
 	val.vector_val = new vector<Val*>();

src/Val.h

@@ -328,6 +328,8 @@ public:
 	void Describe(ODesc* d) const override;
 	virtual void DescribeReST(ODesc* d) const;
 
+	virtual notifier::Modifiable* Modifiable()	{ return 0; }
+
 #ifdef DEBUG
 	// For debugging, we keep a reference to the global ID to which a
 	// value has been bound *last*.
@@ -490,8 +492,6 @@ protected:
 
 	MutableVal()	{}
 	~MutableVal() override;
-
-	void Modified()	{ notifiers.Modified(this); }
 };
 
 #define Microseconds 1e-6
@@ -764,7 +764,7 @@ protected:
 };
 
 class CompositeHash;
-class TableVal : public MutableVal {
+class TableVal : public Val, public notifier::Modifiable {
 public:
 	explicit TableVal(TableType* t, Attributes* attrs = 0);
 	~TableVal() override;
@@ -877,6 +877,8 @@ public:
 	HashKey* ComputeHash(const Val* index) const
 		{ return table_hash->ComputeHash(index, 1); }
 
+	notifier::Modifiable* Modifiable() override	{ return this; }
+
 protected:
 	friend class Val;
 	TableVal()	{}
@@ -915,7 +917,7 @@ protected:
 	Val* def_val;
 };
 
-class RecordVal : public MutableVal {
+class RecordVal : public Val, public notifier::Modifiable {
 public:
 	explicit RecordVal(RecordType* t, bool init_fields = true);
 	~RecordVal() override;
@@ -962,6 +964,8 @@ public:
 	unsigned int MemoryAllocation() const override;
 	void DescribeReST(ODesc* d) const override;
 
+	notifier::Modifiable* Modifiable() override	{ return this; }
+
 	// Extend the underlying arrays of record instances created during
 	// parsing to match the number of fields in the record type (they may
 	// mismatch as a result of parse-time record type redefinitions.
@@ -1006,7 +1010,7 @@ protected:
 };
 
 
-class VectorVal : public MutableVal {
+class VectorVal : public Val, public notifier::Modifiable {
 public:
 	explicit VectorVal(VectorType* t);
 	~VectorVal() override;
@@ -1054,6 +1058,8 @@ public:
 	// Won't shrink size.
 	unsigned int ResizeAtLeast(unsigned int new_num_elements);
 
+	notifier::Modifiable* Modifiable() override	{ return this; }
+
 protected:
 	friend class Val;
 	VectorVal()	{ }