From 11cde53373ce3574a4a2a61b25d6b2b190ae4cfc Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 25 Jul 2022 17:41:12 +0200 Subject: [PATCH] option.bif: Short-circuit option changes when terminating Due to the asynchronous behavior of the input framework and broker communication, change handlers were previously called even after zeek_done() event processing completed and also broker shutdown. Accessing broker store handles within change handlers this late triggered invalid Broker store handle messages: error in ././my_option_store.zeek, line 13: invalid Broker store handle (Broker::put(Test::store, to_any_coercemy_option, to_any_coerceTest::new_value, 0 secs) and broker::store::{}) Fixes #2010 --- NEWS | 4 +++ src/option.bif | 5 ++++ .../Baseline/core.option-zeek-done/.stdout | 7 +++++ testing/btest/core/option-zeek-done.zeek | 29 +++++++++++++++++++ 4 files changed, 45 insertions(+) create mode 100644 testing/btest/Baseline/core.option-zeek-done/.stdout create mode 100644 testing/btest/core/option-zeek-done.zeek diff --git a/NEWS b/NEWS index bd056bc781..9bac110f10 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,10 @@ Changed Functionality - The default logging directory is now set globally across all log writers through ``Log::default_logdir``. +- Calling `Option::set()` when Zeek is terminating is now a noop and returns `F`. + This prevents callbacks into script-land through change handlers when parts + of the environment have already been torn down. + Deprecated Functionality ------------------------ diff --git a/src/option.bif b/src/option.bif index 4996cb8097..8e30cb7dfc 100644 --- a/src/option.bif +++ b/src/option.bif @@ -10,6 +10,11 @@ module Option; static bool call_option_handlers_and_set_value(zeek::StringVal* name, const zeek::detail::IDPtr& i, zeek::ValPtr val, zeek::StringVal* location) { + // when shutting down, don't call back into script land handlers as + // these may use resources that have already been shutdown. + if ( zeek::run_state::terminating ) + return false; + if ( i->HasOptionHandlers() ) { for ( auto handler_function : i->GetOptionHandlers() ) diff --git a/testing/btest/Baseline/core.option-zeek-done/.stdout b/testing/btest/Baseline/core.option-zeek-done/.stdout new file mode 100644 index 0000000000..80ab660fd7 --- /dev/null +++ b/testing/btest/Baseline/core.option-zeek-done/.stdout @@ -0,0 +1,7 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Initial value, T +Value of testbool changed from T to F (zeek_is_terminating=F) +Next value, F +Next changed, T +Final value, F +Final changed, F diff --git a/testing/btest/core/option-zeek-done.zeek b/testing/btest/core/option-zeek-done.zeek new file mode 100644 index 0000000000..ca1af7179f --- /dev/null +++ b/testing/btest/core/option-zeek-done.zeek @@ -0,0 +1,29 @@ +# @TEST-DOC: Ensure change handlers do not run when terminating by trying to change an option during zeek_done() +# @TEST-EXEC: zeek -b %INPUT +# @TEST-EXEC: btest-diff .stdout + +export { + option testbool: bool = T; +} + +function option_changed(ID: string, new_value: bool): bool { + print fmt("Value of %s changed from %s to %s (zeek_is_terminating=%s)", ID, testbool, new_value, zeek_is_terminating()); + return new_value; +} + +event zeek_init() + { + print "Initial value", testbool; + Option::set_change_handler("testbool", option_changed); + local changed = Option::set("testbool", F); + print "Next value", testbool; + print "Next changed", changed; + } + +event zeek_done() + { + local changed = Option::set("testbool", T); + print "Final value", testbool; + print "Final changed", changed; + } +