ctdbd: Avoid freeing non-monitor event callback when monitoring is disabled

When running a non-monitor event, check is made for any active monitor
events.  If there is an active monitor event, then the active monitor
event is cancelled.  This is done by freeing state->callback which is
allocated from monitor_context.

When CTDB is stopped or shutdown, monitoring is disabled by freeing
monitor_context, which frees callback and then stopped or shutdown event
is run.  This creates a new callback structure which is allocated at
the exact same memory location as the monitor callback which was freed.
So in the check for active monitor events, it frees the new callback
for non-monitor event.  Since the callback function flags successful
completion of that event, it is never marked complete and CTDB is stuck
in a loop waiting for completion.

Move the monitor cancellation to the top of the function so that this
can't happen.

Follow log snippest highlights the problem.

2013/04/30 16:54:10.673807 [21505]: Received SHUTDOWN command. Stopping CTDB daemon.
2013/04/30 16:54:10.673814 [21505]: Shutting down recovery daemon
2013/04/30 16:54:10.673852 [21505]: server/eventscript.c:696 in remove_callback 0x1c6d5c0
2013/04/30 16:54:10.673858 [21505]: Monitoring has been stopped
2013/04/30 16:54:10.673899 [21505]: server/eventscript.c:594 Sending SIGTERM to child pid:23847
2013/04/30 16:54:10.673913 [21505]: server/eventscript.c:629 searching for callback 0x1c6d5c0
2013/04/30 16:54:10.673932 [21505]: server/eventscript.c:641 running callback
2013/04/30 16:54:10.673939 [21505]: server/eventscript.c:866 in event_script_callback
2013/04/30 16:54:10.673946 [21505]: server/eventscript.c:696 in remove_callback 0x1c6d5c0

Signed-off-by: Martin Schwenke <martin@meltin.net>
Pair-programmed-with: Amitay Isaacs <amitay@gmail.com>

(This used to be ctdb commit 05f785b51cfd8b22b3ae35bf034127fbc07005be)

1 files changed, 30 insertions, 31 deletions

diff --git a/ctdb/server/eventscript.c b/ctdb/server/eventscript.c
index 817cb0cbc4..41bf21db35 100644
--- a/ctdb/server/eventscript.c
+++ b/ctdb/server/eventscript.c
@@ -707,36 +707,6 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
 {
 	struct ctdb_event_script_state *state;
 
-	state = talloc(ctdb->event_script_ctx, struct ctdb_event_script_state);
-	CTDB_NO_MEMORY(ctdb, state);
-
-	/* The callback isn't done if the context is freed. */
-	state->callback = talloc(mem_ctx, struct event_script_callback);
-	CTDB_NO_MEMORY(ctdb, state->callback);
-	DLIST_ADD(ctdb->script_callbacks, state->callback);
-	talloc_set_destructor(state->callback, remove_callback);
-	state->callback->ctdb         = ctdb;
-	state->callback->fn           = callback;
-	state->callback->private_data = private_data;
-
-	state->ctdb = ctdb;
-	state->from_user = from_user;
-	state->call = call;
-	state->options = talloc_vasprintf(state, fmt, ap);
-	state->timeout = timeval_set(ctdb->tunable.script_timeout, 0);
-	state->scripts = NULL;
-	if (state->options == NULL) {
-		DEBUG(DEBUG_ERR, (__location__ " could not allocate state->options\n"));
-		talloc_free(state);
-		return -1;
-	}
-	if (!check_options(state->call, state->options)) {
-		DEBUG(DEBUG_ERR, ("Bad eventscript options '%s' for %s\n",
-				  ctdb_eventscript_call_names[state->call], state->options));
-		talloc_free(state);
-		return -1;
-	}
-
 	if (ctdb->recovery_mode != CTDB_RECOVERY_NORMAL) {
 		/* we guarantee that only some specifically allowed event scripts are run
 		   while in recovery */
@@ -755,7 +725,6 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
 		if (i == ARRAY_SIZE(allowed_calls)) {
 			DEBUG(DEBUG_ERR,("Refusing to run event scripts call '%s' while in recovery\n",
 				 ctdb_eventscript_call_names[call]));
-			talloc_free(state);
 			return -1;
 		}
 	}
@@ -778,6 +747,36 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
 		ctdb->current_monitor = NULL;
 	}
 
+	state = talloc(ctdb->event_script_ctx, struct ctdb_event_script_state);
+	CTDB_NO_MEMORY(ctdb, state);
+
+	/* The callback isn't done if the context is freed. */
+	state->callback = talloc(mem_ctx, struct event_script_callback);
+	CTDB_NO_MEMORY(ctdb, state->callback);
+	DLIST_ADD(ctdb->script_callbacks, state->callback);
+	talloc_set_destructor(state->callback, remove_callback);
+	state->callback->ctdb         = ctdb;
+	state->callback->fn           = callback;
+	state->callback->private_data = private_data;
+
+	state->ctdb = ctdb;
+	state->from_user = from_user;
+	state->call = call;
+	state->options = talloc_vasprintf(state, fmt, ap);
+	state->timeout = timeval_set(ctdb->tunable.script_timeout, 0);
+	state->scripts = NULL;
+	if (state->options == NULL) {
+		DEBUG(DEBUG_ERR, (__location__ " could not allocate state->options\n"));
+		talloc_free(state);
+		return -1;
+	}
+	if (!check_options(state->call, state->options)) {
+		DEBUG(DEBUG_ERR, ("Bad eventscript options '%s' for %s\n",
+				  ctdb_eventscript_call_names[state->call], state->options));
+		talloc_free(state);
+		return -1;
+	}
+
 	DEBUG(DEBUG_INFO,(__location__ " Starting eventscript %s %s\n",
 			  ctdb_eventscript_call_names[state->call],
 			  state->options));