change Trigger constructor to not potentially auto-delete itself

NEWS

@@ -91,6 +91,15 @@ Deprecated Functionality
 - Accessing globals with ``GLOBAL::name`` has been deprecated and will be
   removed with Zeek 7.1. Use ``::name`` instead.
 
+- The original ``trigger::Trigger`` constructor has been deprecated and will
+  be removed with Zeek 7.1. Use the new alternative constructor
+  (per ``src/Trigger.h``) instead, including replacing any use of ``new ...``
+  with ``make_intrusive<...>``. The new constructor differs only in the
+  placement of the ``timeout`` parameter, and in that - unlike the original -
+  it always returns a valid pointer, which must be Unref()'d after
+  construction, either explicitly (if using ``new``) or implicitly
+  (if using ``make_intrusive<...>``).
+
 Zeek 6.0.0
 ==========
 

src/Stmt.cc

@@ -2268,8 +2268,8 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow)
 			local_aggrs.emplace_back(std::move(v));
 		}
 
-	// The new trigger object will take care of its own deletion.
-	new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, location);
+	(void)make_intrusive<trigger::Trigger>(wi, wi->WhenExprGlobals(), local_aggrs, timeout, f,
+	                                       location);
 
 	return nullptr;
 	}

src/Trigger.cc

@@ -101,6 +101,13 @@ protected:
 
 Trigger::Trigger(std::shared_ptr<WhenInfo> wi, double timeout, const IDSet& _globals,
                  std::vector<ValPtr> _local_aggrs, Frame* f, const Location* loc)
+	: Trigger(std::move(wi), _globals, std::move(_local_aggrs), timeout, f, loc)
+	{
+	Unref(this);
+	}
+
+Trigger::Trigger(std::shared_ptr<WhenInfo> wi, const IDSet& _globals,
+                 std::vector<ValPtr> _local_aggrs, double timeout, Frame* f, const Location* loc)
 	{
 	timeout_value = timeout;
 	globals = _globals;
@@ -117,8 +124,8 @@ Trigger::Trigger(std::shared_ptr<WhenInfo> wi, double timeout, const IDSet& _glo
 	disabled = false;
 	attached = nullptr;
 
-	if ( location )
-		name = util::fmt("%s:%d-%d", location->filename, location->first_line, location->last_line);
+	if ( loc )
+		name = util::fmt("%s:%d-%d", loc->filename, loc->first_line, loc->last_line);
 	else
 		name = "<no-trigger-location>";
 
@@ -135,7 +142,6 @@ Trigger::Trigger(std::shared_ptr<WhenInfo> wi, double timeout, const IDSet& _glo
 		if ( ! parent )
 			{
 			reporter->Error("return trigger in context which does not allow delaying result");
-			Unref(this);
 			return;
 			}
 
@@ -152,8 +158,6 @@ Trigger::Trigger(std::shared_ptr<WhenInfo> wi, double timeout, const IDSet& _glo
 		timer = new TriggerTimer(timeout_value, this);
 		timer_mgr->Add(timer);
 		}
-
-	Unref(this);
 	}
 
 void Trigger::Terminate()

src/Trigger.h

@@ -43,13 +43,33 @@ class TriggerTraversalCallback;
 class Trigger final : public Obj, public notifier::detail::Receiver
 	{
 public:
-	// Don't access Trigger objects; they take care of themselves after
-	// instantiation.  Note that if the condition is already true, the
-	// statements are executed immediately and the object is deleted
-	// right away.
+	// This first constructor can return an invalid pointer, so
+	// its value must not be used further.
+	[[deprecated("Remove in v7.1.  Use second Trigger constructor via "
+	             "make_intrusive<...>.")]] Trigger(std::shared_ptr<WhenInfo> wi, double timeout,
+	                                               const IDSet& globals,
+	                                               std::vector<ValPtr> local_aggrs, Frame* f,
+	                                               const Location* loc);
 
-	Trigger(std::shared_ptr<WhenInfo> wi, double timeout, const IDSet& globals,
-	        std::vector<ValPtr> local_aggrs, Frame* f, const Location* loc);
+	// Use this constructor via make_intrusive<...>. The usual pattern is
+	// to then discard what's returned, i.e. "(void)make_intrusive<...>" -
+	// however, a valid pointer will be returned that can be used for
+	// subsequent method calls.
+	//
+	// The reason for this complexity is that if the trigger condition
+	// is true upon construction, after construction finishes there's
+	// no more to do and the object should be deleted (Unref()'d).
+	// If the condition is not true, then the constructor ensures that
+	// the object will be tracked via sufficient Ref()'ing for further
+	// processing and eventual deletion once the trigger is satisfied
+	// or times out.
+	//
+	// Note that this constructor differs from the deprecated one only
+	// in where the "timeout" parameter appears, and in making the "loc"
+	// parameter optional. ("loc" is only used for internal logging when
+	// debugging triggers.)
+	Trigger(std::shared_ptr<WhenInfo> wi, const IDSet& globals, std::vector<ValPtr> local_aggrs,
+	        double timeout, Frame* f, const Location* loc = nullptr);
 
 	~Trigger() override;
 
@@ -104,7 +124,6 @@ public:
 	void Terminate() override;
 
 	const char* Name() const { return name.c_str(); }
-	void SetName(const char* new_name) { name = new_name; }
 
 private:
 	friend class TriggerTimer;

src/script_opt/CPP/Stmts.cc

@@ -438,14 +438,10 @@ void CPPCompile::GenWhenStmt(const WhenStmt* w)
 	Emit("new_frame->SetTrigger({NewRef{}, curr_t});");
 	Emit("new_frame->SetTriggerAssoc(curr_assoc);");
 
-	Emit("auto t = new trigger::Trigger(CPP__wi, %s, CPP__w_globals, CPP__local_aggrs, "
-	     "new_frame.get(), "
-	     "nullptr);",
+	Emit("auto t = make_intrusive<trigger::Trigger>(CPP__wi, CPP__w_globals, CPP__local_aggrs, %s, "
+	     "new_frame.get());",
 	     timeout_val);
 
-	auto loc_str = util::fmt("%s:%d-%d", loc->filename, loc->first_line, loc->last_line);
-	Emit("t->SetName(\"%s\");", loc_str);
-
 	if ( ret_type && ret_type->Tag() != TYPE_VOID )
 		{
 		// Note, ret_type can be active but we *still* don't have

src/script_opt/ZAM/Ops.in

@@ -2079,7 +2079,7 @@ macro BuildWhen(timeout)
 		if ( v )
 			local_aggrs.push_back(v);
 		}
-	new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, z.loc);
+	(void)make_intrusive<trigger::Trigger>(wi, wi->WhenExprGlobals(), local_aggrs, timeout, f, z.loc);
 
 ########################################
 # Internal