Merge branch 'topic/jgras/expire-redef' of https://github.com/J-Gras/bro

Extended error handling a bit, and increased serialization
data format version.

CHANGES

@@ -1,4 +1,11 @@
 
+2.4-609 | 2016-06-14 17:15:28 -0700
+
+  * Fixed table expiration evaluation. The expiration attribute
+    expression is now evaluated for every use. Thus later adjustments
+    of the value (e.g. by redefining a const) will now take effect.
+    Values less than 0 will disable expiration. (Jan Grashoefer)
+
 2.4-606 | 2016-06-14 16:11:07 -0700
 
   * Fix parsing precedence of "hook" expression. Addresses BIT-1619

VERSION

@@ -1 +1 @@
-2.4-606
+2.4-609

aux/broccoli

@@ -1 +1 @@
-Subproject commit 65cf386961f5e37f6898d372dd7a5a949b5eaa28
+Subproject commit 2592077f96008f5c64b23b6fd605bfce3ec47d84

src/Serializer.h

@@ -125,7 +125,7 @@ protected:
 
 	// This will be increased whenever there is an incompatible change
 	// in the data format.
-	static const uint32 DATA_FORMAT_VERSION = 25;
+	static const uint32 DATA_FORMAT_VERSION = 26;
 
 	ChunkedIO* io;
 

src/Val.cc

@@ -1323,7 +1323,7 @@ void TableVal::Init(TableType* t)
 	{
 	::Ref(t);
 	table_type = t;
-	expire_expr = 0;
+	expire_func = 0;
 	expire_time = 0;
 	expire_cookie = 0;
 	timer = 0;
@@ -1350,7 +1350,8 @@ TableVal::~TableVal()
 	delete subnets;
 	Unref(attrs);
 	Unref(def_val);
-	Unref(expire_expr);
+	Unref(expire_func);
+	Unref(expire_time);
 	}
 
 void TableVal::RemoveAll()
@@ -1399,8 +1400,8 @@ void TableVal::SetAttrs(Attributes* a)
 	Attr* ef = attrs->FindAttr(ATTR_EXPIRE_FUNC);
 	if ( ef )
 		{
-		expire_expr = ef->AttrExpr();
-		expire_expr->Ref();
+		expire_func = ef->AttrExpr();
+		expire_func->Ref();
 		}
 	}
 
@@ -1410,15 +1411,17 @@ void TableVal::CheckExpireAttr(attr_tag at)
 
 	if ( a )
 		{
-		Val* timeout = a->AttrExpr()->Eval(0);
-		if ( ! timeout )
+		expire_time = a->AttrExpr();
+		expire_time->Ref();
+
+		if ( expire_time->Type()->Tag() != TYPE_INTERVAL )
 			{
-			a->AttrExpr()->Error("value of timeout not fixed");
+			if ( ! expire_time->IsError() )
+				expire_time->SetError("expiration interval has wrong type");
+
 			return;
 			}
 
-		expire_time = timeout->AsInterval();
-
 		if ( timer )
 			timer_mgr->Cancel(timer);
 
@@ -1791,7 +1794,7 @@ Val* TableVal::Lookup(Val* index, bool use_default_val)
 			if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) )
 					{
 					v->SetExpireAccess(network_time);
-					if ( LoggingAccess() && expire_time )
+					if ( LoggingAccess() && ExpirationEnabled() )
 						ReadOperation(index, v);
 					}
 
@@ -1822,7 +1825,7 @@ Val* TableVal::Lookup(Val* index, bool use_default_val)
 				if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) )
 					{
 					v->SetExpireAccess(network_time);
-					if ( LoggingAccess() && expire_time )
+					if ( LoggingAccess() && ExpirationEnabled() )
 						ReadOperation(index, v);
 					}
 
@@ -1880,7 +1883,7 @@ TableVal* TableVal::LookupSubnetValues(const SubNetVal* search)
 			if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) )
 				{
 				entry->SetExpireAccess(network_time);
-				if ( LoggingAccess() && expire_time )
+				if ( LoggingAccess() && ExpirationEnabled() )
 					ReadOperation(s, entry);
 				}
 			}
@@ -2176,6 +2179,13 @@ void TableVal::DoExpire(double t)
 
 	PDict(TableEntryVal)* tbl = AsNonConstTable();
 
+	double timeout = GetExpireTime();
+
+	if ( timeout < 0 )
+		// Skip in case of unset/invalid expiration value. If it's an
+		// error, it has been reported already.
+		return;
+
 	if ( ! expire_cookie )
 		{
 		expire_cookie = tbl->InitForIteration();
@@ -2197,11 +2207,11 @@ void TableVal::DoExpire(double t)
 			// correct, so we just need to wait.
 			}
 
-		else if ( v->ExpireAccessTime() + expire_time < t )
+		else if ( v->ExpireAccessTime() + timeout < t )
 			{
 			Val* val = v->Value();
 
-			if ( expire_expr )
+			if ( expire_func )
 				{
 				Val* idx = RecoverIndex(k);
 				double secs = CallExpireFunc(idx);
@@ -2221,7 +2231,7 @@ void TableVal::DoExpire(double t)
 					{
 					// User doesn't want us to expire
 					// this now.
-					v->SetExpireAccess(network_time - expire_time + secs);
+					v->SetExpireAccess(network_time - timeout + secs);
 					delete k;
 					continue;
 					}
@@ -2258,9 +2268,27 @@ void TableVal::DoExpire(double t)
 		InitTimer(table_expire_delay);
 	}
 
+double TableVal::GetExpireTime()
+	{
+	if ( ! expire_time )
+		return -1;
+
+	Val* timeout = expire_time->Eval(0);
+
+	if ( timeout && (timeout->AsInterval() >= 0) )
+		return timeout->AsInterval();
+
+	expire_time = 0;
+
+	if ( timer )
+		timer_mgr->Cancel(timer);
+
+	return -1;
+	}
+
 double TableVal::CallExpireFunc(Val* idx)
 	{
-	if ( ! expire_expr )
+	if ( ! expire_func )
 		{
 		Unref(idx);
 		return 0;
@@ -2285,7 +2313,7 @@ double TableVal::CallExpireFunc(Val* idx)
 
 	try
 		{
-		Val* vf = expire_expr->Eval(0);
+		Val* vf = expire_func->Eval(0);
 
 		if ( ! vf )
 			{
@@ -2319,11 +2347,18 @@ double TableVal::CallExpireFunc(Val* idx)
 
 void TableVal::ReadOperation(Val* index, TableEntryVal* v)
 	{
+	double timeout = GetExpireTime();
+
+	if ( timeout < 0 )
+		// Skip in case of unset/invalid expiration value. If it's an
+		// error, it has been reported already.
+		return;
+
 	// In theory we need to only propagate one update per &read_expire
 	// interval to prevent peers from expiring intervals. To account for
 	// practical issues such as latency, we send one update every half
 	// &read_expire.
-	if ( network_time - v->LastReadUpdate() > expire_time / 2 )
+	if ( network_time - v->LastReadUpdate() > timeout / 2 )
 		{
 		StateAccess::Log(new StateAccess(OP_READ_IDX, this, index));
 		v->SetLastReadUpdate(network_time);
@@ -2362,11 +2397,9 @@ bool TableVal::DoSerialize(SerialInfo* info) const
 		state->did_index = false;
 		info->s->WriteOpenTag(table_type->IsSet() ? "set" : "table");
 
-		if ( ! SERIALIZE(expire_time) )
-			return false;
-
 		SERIALIZE_OPTIONAL(attrs);
-		SERIALIZE_OPTIONAL(expire_expr);
+		SERIALIZE_OPTIONAL(expire_time);
+		SERIALIZE_OPTIONAL(expire_func);
 
 		// Make sure nobody kills us in between.
 		const_cast<TableVal*>(this)->Ref();
@@ -2491,13 +2524,11 @@ bool TableVal::DoUnserialize(UnserialInfo* info)
 	{
 	DO_UNSERIALIZE(MutableVal);
 
-	if ( ! UNSERIALIZE(&expire_time) )
-		return false;
-
 	Init((TableType*) type);
 
 	UNSERIALIZE_OPTIONAL(attrs, Attributes::Unserialize(info));
-	UNSERIALIZE_OPTIONAL(expire_expr, Expr::Unserialize(info));
+	UNSERIALIZE_OPTIONAL(expire_time, Expr::Unserialize(info));
+	UNSERIALIZE_OPTIONAL(expire_func, Expr::Unserialize(info));
 
 	while ( true )
 		{

src/Val.h

@@ -644,7 +644,7 @@ protected:
 	DECLARE_SERIAL(PatternVal);
 };
 
-// ListVals are mainly used to index tables that have more than one 
+// ListVals are mainly used to index tables that have more than one
 // element in their index.
 class ListVal : public Val {
 public:
@@ -862,6 +862,14 @@ protected:
 	// Calculates default value for index.  Returns 0 if none.
 	Val* Default(Val* index);
 
+	// Returns true if item expiration is enabled.
+	bool ExpirationEnabled()	{ return expire_time != 0; }
+
+	// Returns the expiration time defined by %{create,read,write}_expire
+	// attribute, or -1 for unset/invalid values. In the invalid case, an
+	// error will have been reported.
+	double GetExpireTime();
+
 	// Calls &expire_func and returns its return interval;
 	// takes ownership of the reference.
 	double CallExpireFunc(Val *idx);
@@ -874,8 +882,8 @@ protected:
 	TableType* table_type;
 	CompositeHash* table_hash;
 	Attributes* attrs;
-	double expire_time;
-	Expr* expire_expr;
+	Expr* expire_time;
+	Expr* expire_func;
 	TableValTimer* timer;
 	IterCookie* expire_cookie;
 	PrefixTable* subnets;

testing/btest/Baseline/language.expire-expr-error/output

@@ -0,0 +1,2 @@
+error in /home/robin/bro/master/testing/btest/.tmp/language.expire-expr-error/expire-expr-error.bro, line 7: no such index (x[kaputt])
+received termination signal

testing/btest/Baseline/language.expire-redef/output

@@ -0,0 +1,5 @@
+Run 0
+Run 1
+Run 2
+Expired: 0 --> some data
+Run 3

testing/btest/Baseline/language.expire-type-error/out

@@ -0,0 +1 @@
+error in /home/robin/bro/master/testing/btest/.tmp/language.expire-type-error/expire-type-error.bro, line 4: expiration interval has wrong type (kaputt)

testing/btest/language/expire-expr-error.bro

@@ -0,0 +1,29 @@
+# @TEST-EXEC: btest-bg-run broproc bro %INPUT
+# @TEST-EXEC: btest-bg-wait -k 5
+# @TEST-EXEC: cat broproc/.stderr > output
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output
+
+global x: table[string] of interval;
+global data: table[int] of string &create_expire=x["kaputt"];
+
+@load frameworks/communication/listen
+
+global runs = 0;
+event do_it()
+	{
+	print fmt("Run %s", runs);
+
+	++runs;
+	if ( runs < 4 )
+		schedule 1sec { do_it() };
+	}
+
+
+event bro_init() &priority=-10
+	{
+	data[0] = "some data";
+	schedule 1sec { do_it() };
+	}
+
+
+

testing/btest/language/expire-redef.bro

@@ -0,0 +1,38 @@
+# @TEST-EXEC: btest-bg-run broproc bro %INPUT
+# @TEST-EXEC: btest-bg-wait -k 5
+# @TEST-EXEC: cat broproc/.stdout > output
+# @TEST-EXEC: btest-diff output
+
+
+@load frameworks/communication/listen
+
+const exp_val = -1sec &redef;
+
+global expired: function(tbl: table[int] of string, idx: int): interval;
+global data: table[int] of string &write_expire=exp_val &expire_func=expired;
+
+redef table_expire_interval = 1sec;
+redef exp_val = 3sec;
+
+global runs = 0;
+event do_it()
+	{
+	print fmt("Run %s", runs);
+
+	++runs;
+	if ( runs < 4 )
+		schedule 1sec { do_it() };
+	}
+
+
+function expired(tbl: table[int] of string, idx: int): interval
+	{
+	print fmt("Expired: %s --> %s", idx, tbl[idx]);
+	return 0sec;
+	}
+
+event bro_init() &priority=-10
+	{
+	data[0] = "some data";
+	schedule 1sec { do_it() };
+	}

testing/btest/language/expire-type-error.bro

@@ -0,0 +1,6 @@
+# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+global data: table[int] of string &write_expire="kaputt";
+
+