From 277e6d4129b193a9cf3a0268108ee07a138428ab Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 27 Mar 2025 10:40:24 +0100 Subject: [PATCH 1/6] Event: Header cleanup --- src/Event.cc | 7 ++----- src/Event.h | 4 +--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Event.cc b/src/Event.cc index e07b5d4e4b..6f2a1f9a89 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -2,17 +2,14 @@ #include "zeek/Event.h" -#include "zeek/zeek-config.h" - #include "zeek/Desc.h" -#include "zeek/Func.h" -#include "zeek/NetVar.h" #include "zeek/Trigger.h" #include "zeek/Val.h" #include "zeek/iosource/Manager.h" -#include "zeek/iosource/PktSrc.h" #include "zeek/plugin/Manager.h" +#include "event.bif.netvar_h" + zeek::EventMgr zeek::event_mgr; namespace zeek { diff --git a/src/Event.h b/src/Event.h index a1789aed87..7283ce4645 100644 --- a/src/Event.h +++ b/src/Event.h @@ -5,12 +5,10 @@ #include #include -#include "zeek/Flare.h" -#include "zeek/IntrusivePtr.h" #include "zeek/ZeekArgs.h" -#include "zeek/ZeekList.h" #include "zeek/analyzer/Analyzer.h" #include "zeek/iosource/IOSource.h" +#include "zeek/util.h" namespace zeek { From bef923ebeb0d147a5e933c38753d0ae15dcb19dd Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 27 Mar 2025 10:41:01 +0100 Subject: [PATCH 2/6] EventMgr: Drop src_val This is a left over and hasn't been used since a while. --- src/Event.cc | 3 --- src/Event.h | 1 - 2 files changed, 4 deletions(-) diff --git a/src/Event.cc b/src/Event.cc index 6f2a1f9a89..678b81b47f 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -69,7 +69,6 @@ EventMgr::EventMgr() { current_src = util::detail::SOURCE_LOCAL; current_aid = 0; current_ts = 0; - src_val = nullptr; draining = false; } @@ -79,8 +78,6 @@ EventMgr::~EventMgr() { Unref(head); head = n; } - - Unref(src_val); } void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID src, analyzer::ID aid, Obj* obj, diff --git a/src/Event.h b/src/Event.h index 7283ce4645..44c47048e6 100644 --- a/src/Event.h +++ b/src/Event.h @@ -123,7 +123,6 @@ protected: util::detail::SourceID current_src; analyzer::ID current_aid; double current_ts; - RecordVal* src_val; bool draining; }; From 7dadbb0c1d339e291926a8ee6838cd7197e46577 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 27 Mar 2025 11:10:08 +0100 Subject: [PATCH 3/6] EventMgr: Do not cache current event attributes Avoid proliferation of various members on EventMgr by storing the pointer of the current event instead. This subtly changes the behavior of some builtin functions as they would have returned the prior event's data when executed outside of event draining (e.g. C++ level hook invocations), but I think that's actually for the better. --- NEWS | 5 +++++ src/Event.cc | 34 +++++++++++----------------------- src/Event.h | 26 ++++++++++++-------------- src/zeek.bif | 4 ++-- 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/NEWS b/NEWS index 8c606ea38b..7feedfb65c 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,11 @@ Zeek 7.2.0 Breaking Changes ---------------- +- The ``is_remote_event()``, ``current_analyzer()`` and ``current_event_time()`` + builtin functions do not return the previous event's values anymore when event + draining has completed. The same applies to the corresponding C++ accessors on + the ``EventMgr`` class. The functions now return false, 0 or the zero time instead. + New Functionality ----------------- diff --git a/src/Event.cc b/src/Event.cc index 678b81b47f..4525118de6 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -64,14 +64,6 @@ void Event::Dispatch(bool no_remote) { reporter->EndErrorHandler(); } -EventMgr::EventMgr() { - head = tail = nullptr; - current_src = util::detail::SOURCE_LOCAL; - current_aid = 0; - current_ts = 0; - draining = false; -} - EventMgr::~EventMgr() { while ( head ) { Event* n = head->NextEvent(); @@ -103,10 +95,10 @@ void EventMgr::QueueEvent(Event* event) { } void EventMgr::Dispatch(Event* event, bool no_remote) { - current_src = event->Source(); - current_aid = event->Analyzer(); - current_ts = event->Time(); + Event* old_current = current; + current = event; event->Dispatch(no_remote); + current = old_current; Unref(event); } @@ -116,8 +108,6 @@ void EventMgr::Drain() { PLUGIN_HOOK_VOID(HOOK_DRAIN_EVENTS, HookDrainEvents()); - draining = true; - // Past Zeek versions drained as long as there events, including when // a handler queued new events during its execution. This could lead // to endless loops in case a handler kept triggering its own event. @@ -126,27 +116,25 @@ void EventMgr::Drain() { // that expect the old behavior to trigger something quickly. for ( int round = 0; head && round < 2; round++ ) { - Event* current = head; + Event* event = head; head = nullptr; tail = nullptr; - while ( current ) { - Event* next = current->NextEvent(); + while ( event ) { + Event* next = event->NextEvent(); - current_src = current->Source(); - current_aid = current->Analyzer(); - current_ts = current->Time(); - current->Dispatch(); - Unref(current); + current = event; + event->Dispatch(); + Unref(event); ++event_mgr.num_events_dispatched; - current = next; + event = next; } } // Note: we might eventually need a general way to specify things to // do after draining events. - draining = false; + current = nullptr; // Make sure all of the triggers get processed every time the events // drain. diff --git a/src/Event.h b/src/Event.h index 44c47048e6..76c761945f 100644 --- a/src/Event.h +++ b/src/Event.h @@ -52,7 +52,6 @@ protected: class EventMgr final : public Obj, public iosource::IOSource { public: - EventMgr(); ~EventMgr() override; /** @@ -84,21 +83,23 @@ public: void Dispatch(Event* event, bool no_remote = false); void Drain(); - bool IsDraining() const { return draining; } + bool IsDraining() const { return current != nullptr; } bool HasEvents() const { return head != nullptr; } - // Returns the source ID of last raised event. - util::detail::SourceID CurrentSource() const { return current_src; } + // Returns the source ID of the current event. + util::detail::SourceID CurrentSource() const { return current ? current->Source() : util::detail::SOURCE_LOCAL; } - // Returns the ID of the analyzer which raised the last event, or 0 if + // Returns the ID of the analyzer which raised the current event, or 0 if // non-analyzer event. - analyzer::ID CurrentAnalyzer() const { return current_aid; } + analyzer::ID CurrentAnalyzer() const { return current ? current->Analyzer() : 0; } - // Returns the timestamp of the last raised event. The timestamp reflects the network time + // Returns the timestamp of the current event. The timestamp reflects the network time // the event was intended to be executed. For scheduled events, this is the time the event // was scheduled to. For any other event, this is the time when the event was created. - double CurrentEventTime() const { return current_ts; } + // + // If no event is being processed, returns 0.0. + double CurrentEventTime() const { return current ? current->Time() : 0.0; } int Size() const { return num_events_queued - num_events_dispatched; } @@ -118,12 +119,9 @@ public: protected: void QueueEvent(Event* event); - Event* head; - Event* tail; - util::detail::SourceID current_src; - analyzer::ID current_aid; - double current_ts; - bool draining; + Event* current = nullptr; + Event* head = nullptr; + Event* tail = nullptr; }; extern EventMgr event_mgr; diff --git a/src/zeek.bif b/src/zeek.bif index e97c988d39..7c5fac0f09 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -4973,9 +4973,9 @@ function uninstall_dst_net_filter%(snet: subnet%) : bool return zeek::val_mgr->Bool(packet_mgr->GetPacketFilter()->RemoveDst(snet)); %} -## Checks whether the last raised event came from a remote peer. +## Checks whether the current event came from a remote peer. ## -## Returns: True if the last raised event came from a remote peer. +## Returns: True if the current event came from a remote peer. function is_remote_event%(%) : bool %{ return zeek::val_mgr->Bool(zeek::event_mgr.CurrentSource() != zeek::util::detail::SOURCE_LOCAL); From b535f033825ee0715f7e701d20e373342fe8841a Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 27 Mar 2025 14:30:57 +0100 Subject: [PATCH 4/6] EventHandler: Header cleanup --- src/EventHandler.cc | 1 - src/EventHandler.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/EventHandler.cc b/src/EventHandler.cc index c375b3a3e1..b6f11a0b53 100644 --- a/src/EventHandler.cc +++ b/src/EventHandler.cc @@ -6,7 +6,6 @@ #include "zeek/Event.h" #include "zeek/Func.h" #include "zeek/ID.h" -#include "zeek/NetVar.h" #include "zeek/Scope.h" #include "zeek/Var.h" #include "zeek/broker/Data.h" diff --git a/src/EventHandler.h b/src/EventHandler.h index f6da3378fa..cb612eb825 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -4,13 +4,11 @@ #pragma once -#include #include #include #include "zeek/Type.h" #include "zeek/ZeekArgs.h" -#include "zeek/ZeekList.h" namespace zeek { From f7425b805d5232a3acf998587916e707e5f5bd19 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 26 Mar 2025 18:11:25 +0100 Subject: [PATCH 5/6] EventHandler: Deprecate Call(args, no_remote, ts), add Call(args) The ts parameter was only added to Call() for the Broker::auto_publish() functionality and propagating the network timestamp. By now, the auto-publish functionality is deprecated, so it'd be good to cleanup that signature. There won't be any need for no_remote in the future either. Allow users to just use Call() instead. --- src/Event.cc | 4 ++++ src/EventHandler.h | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/Event.cc b/src/Event.cc index 4525118de6..6d449684e5 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -49,7 +49,11 @@ void Event::Dispatch(bool no_remote) { reporter->BeginErrorHandler(); try { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + // Replace in v8.1 with handler->Call(&args). handler->Call(&args, no_remote, ts); +#pragma GCC diagnostic pop } catch ( InterpreterException& e ) { diff --git a/src/EventHandler.h b/src/EventHandler.h index cb612eb825..5f8472b2ce 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -45,8 +45,19 @@ public: auto_publish.erase(topic); } + [[deprecated( + "Remove in v8.1. The no_remote and ts parameters are AutoPublish() specific and won't have an effect " + "in the future. Use Call(args)")]] void Call(zeek::Args* vl, bool no_remote = false, double ts = run_state::network_time); + // Call the function associated with this handler. + void Call(zeek::Args* vl) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + Call(vl, false, run_state::network_time); +#pragma GCC diagnostic pop + } + // Returns true if there is at least one local or remote handler. explicit operator bool() const; From 2f9b1e21bdf58a66f7bbc94fb7e89101d6a3a883 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 27 Mar 2025 14:38:17 +0100 Subject: [PATCH 6/6] Event/EventMgr: protected to private These classes are final, so deriving isn't possible. No reason to have protected members. --- src/Event.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Event.h b/src/Event.h index 76c761945f..078595a7fe 100644 --- a/src/Event.h +++ b/src/Event.h @@ -34,7 +34,7 @@ public: void Describe(ODesc* d) const override; -protected: +private: friend class EventMgr; // This method is protected to make sure that everybody goes through @@ -116,7 +116,7 @@ public: uint64_t num_events_queued = 0; uint64_t num_events_dispatched = 0; -protected: +private: void QueueEvent(Event* event); Event* current = nullptr;