From d158c7ffdfed8ac58e107efaba660a1d541cb7d0 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 19 Feb 2013 16:19:16 -0600 Subject: [PATCH] Fix memory leaks resulting from 'when' and 'return when' statements. Addresses #946. --- src/Frame.cc | 1 + src/StateAccess.cc | 13 +++- src/Trigger.cc | 10 ++- src/Trigger.h | 2 +- src/Type.cc | 2 + testing/btest/core/leaks/returnwhen.bro | 84 +++++++++++++++++++++++++ 6 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 testing/btest/core/leaks/returnwhen.bro diff --git a/src/Frame.cc b/src/Frame.cc index f86fa32805..8a21b02b9e 100644 --- a/src/Frame.cc +++ b/src/Frame.cc @@ -87,6 +87,7 @@ Frame* Frame::Clone() void Frame::SetTrigger(Trigger* arg_trigger) { + ClearTrigger(); if ( arg_trigger ) Ref(arg_trigger); trigger = arg_trigger; diff --git a/src/StateAccess.cc b/src/StateAccess.cc index 2d0a8dfc5a..0deaef5bb5 100644 --- a/src/StateAccess.cc +++ b/src/StateAccess.cc @@ -926,17 +926,22 @@ void NotifierRegistry::Register(ID* id, NotifierRegistry::Notifier* notifier) DBG_LOG(DBG_NOTIFIERS, "registering ID %s for notifier %s", id->Name(), notifier->Name()); + Attr* attr = new Attr(ATTR_TRACKED); + if ( id->Attrs() ) - id->Attrs()->AddAttr(new Attr(ATTR_TRACKED)); + { + if ( ! id->Attrs()->FindAttr(ATTR_TRACKED) ) + id->Attrs()->AddAttr(attr); + } else { attr_list* a = new attr_list; - Attr* attr = new Attr(ATTR_TRACKED); a->append(attr); id->SetAttrs(new Attributes(a, id->Type(), false)); - Unref(attr); } + Unref(attr); + NotifierMap::iterator i = ids.find(id->Name()); if ( i != ids.end() ) @@ -967,7 +972,9 @@ void NotifierRegistry::Unregister(ID* id, NotifierRegistry::Notifier* notifier) if ( i == ids.end() ) return; + Attr* attr = id->Attrs()->FindAttr(ATTR_TRACKED); id->Attrs()->RemoveAttr(ATTR_TRACKED); + Unref(attr); NotifierSet* s = i->second; s->erase(notifier); diff --git a/src/Trigger.cc b/src/Trigger.cc index e7e887e055..a567528232 100644 --- a/src/Trigger.cc +++ b/src/Trigger.cc @@ -242,6 +242,7 @@ bool Trigger::Eval() trigger->Cache(frame->GetCall(), v); trigger->Release(); + frame->ClearTrigger(); } Unref(v); @@ -251,7 +252,6 @@ bool Trigger::Eval() timer_mgr->Cancel(timer); Disable(); - UnregisterAll(); Unref(this); return true; @@ -331,6 +331,7 @@ void Trigger::Timeout() #endif trigger->Cache(frame->GetCall(), v); trigger->Release(); + frame->ClearTrigger(); } Unref(v); @@ -338,7 +339,6 @@ void Trigger::Timeout() } Disable(); - UnregisterAll(); Unref(this); } @@ -426,6 +426,12 @@ Val* Trigger::Lookup(const CallExpr* expr) return (i != cache.end()) ? i->second : 0; } +void Trigger::Disable() + { + UnregisterAll(); + disabled = true; + } + const char* Trigger::Name() const { assert(location); diff --git a/src/Trigger.h b/src/Trigger.h index dd2e171ef6..b752ea8ada 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -49,7 +49,7 @@ public: // Disable this trigger completely. Needed because Unref'ing the trigger // may not immediately delete it as other references may still exist. - void Disable() { disabled = true; } + void Disable(); virtual void Describe(ODesc* d) const { d->Add(""); } diff --git a/src/Type.cc b/src/Type.cc index 1fb813efa1..6ac6070660 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -696,7 +696,9 @@ string FuncType::FlavorString() const FuncType::~FuncType() { + Unref(args); Unref(arg_types); + Unref(yield); } BroType* FuncType::YieldType() diff --git a/testing/btest/core/leaks/returnwhen.bro b/testing/btest/core/leaks/returnwhen.bro new file mode 100644 index 0000000000..9fd9a794cd --- /dev/null +++ b/testing/btest/core/leaks/returnwhen.bro @@ -0,0 +1,84 @@ +# Needs perftools support. +# +# @TEST-GROUP: leaks +# +# @TEST-REQUIRES: bro --help 2>&1 | grep -q mem-leaks +# +# @TEST-EXEC: btest-bg-run bro HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -m -b %INPUT +# @TEST-EXEC: btest-bg-wait 15 + +redef exit_only_after_terminate = T; + +global my_set: set[string] = set(); +global flag: string = "flag"; +global done: bool = F; + +function dummyfunc(s: string): string + { + return "dummy " + s; + } + +function async_func(s: string): string + { + print dummyfunc("from async_func() " + s); + + return when ( flag in my_set ) + { + return flag + " in my_set"; + } + timeout 3sec + { + return "timeout"; + } + } + +event set_flag() + { + add my_set[flag]; + } + +event do_another() + { + delete my_set[flag]; + + local local_dummy = dummyfunc; + + local anon = function(s: string): string { return s + "!"; }; + + if ( ! done ) + schedule 1sec { set_flag() }; + + when ( local result = async_func("from do_another()") ) + { + print "async_func() return result in do_another()", result; + print local_dummy("from do_another() when block"); + print anon("hi"); + if ( result == "timeout" ) + terminate(); + else + { + done = T; + schedule 10msec { do_another() }; + } + } + } + +event bro_init() + { + local local_dummy = dummyfunc; + + local anon = function(s: string): string { return s + "!"; }; + + schedule 1sec { set_flag() }; + + when ( local result = async_func("from bro_init()") ) + { + print "async_func() return result in bro_init()", result; + print local_dummy("from bro_init() when block"); + print anon("hi"); + if ( result == "timeout" ) terminate(); + schedule 10msec { do_another() }; + } + } + +