Option change handler invoked after zeek_done() event

[awelzel]

In a package of ours we are accessing a Broker::Store handle in a change handler. We got bitten by "invalid Broker store handle" when the change handler is invoked during/after shutdown.

Chatting with @rsmmr / @ckreibich about change handlers and broker stores, there was slight surprise to hear that these are invoked after zeek_done() and it was suggested to open a ticket.

The following reduced script shows the issue we observe.

redef Config::config_files += { "./my_option.dat" };
module Test;

global store: opaque of Broker::Store;

export {
    option my_option = "test";
}

function change_handler(id: string, new_value: string): string {
  print(fmt("%s change_handler() zeek_is_terminating=%s store_is_closed=%s store=%s", current_time(), zeek_is_terminating(), Broker::is_closed(store), store));
  Broker::put(store, "my_option", new_value);
  return new_value;
}

event zeek_done() {
  print(fmt("%s zeek_done() zeek_is_terminating=%s store_is_closed=%s store=%s", current_time(), zeek_is_terminating(), Broker::is_closed(store), store));
}

event zeek_init() {
  Option::set_change_handler("Test::my_option", change_handler);
  store = Broker::create_master("my_store");
  print(fmt("%s zeek_init() store_is_closed=%s store=%s ", current_time(), Broker::is_closed(store), store));
}

my_option.dat

Test::my_option updated

$ zeek ./my_option_store.zeek 
1648205596.193555 zeek_init() store_is_closed=F store=broker::store::{my_store} 
1648205596.194328 zeek_done() zeek_is_terminating=T store_is_closed=F store=broker::store::{my_store}
1648205596.194757 change_handler() zeek_is_terminating=T store_is_closed=T store=broker::store::{}
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::{})

---

As an aside, it could be argued that the above script should just rely on option/configuration framework (agree, but it's also the reduced version). But change handlers being invoked after zeek_done() has happened still seems a vaild issue to discuss.

[0xxon]

Yup, seems like a bug - change handlers probably should not be executing when zeek_is_terminating=T.