From b0e9a91f115eaf384714eafb492563c20d7cb6fb Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Tue, 26 Nov 2013 22:25:19 +0400
Subject: [PATCH] space-luatriggers, gh-40, code review

Use common context for lua reference and for trigger state.
Wrap all calls to lua_call into a try/catch block.
Introduce LuarefGuard
Remove box_luactx, it's not adding any value now.
Implement multiple triggers. Reuse code in lua/trigger.cc.
Misc cleanups.
Rename a few files.
---
 include/errcode.h                  |   2 +-
 include/lua/trigger.h              |  69 +++++++++++++
 include/trigger.h                  |   8 --
 src/CMakeLists.txt                 |   1 +
 src/box/alter.cc                   |   7 +-
 src/box/box_lua.cc                 |  25 -----
 src/box/box_lua.h                  |   6 --
 src/box/box_lua_space.cc           | 158 ++++-------------------------
 src/lua/session.cc                 | 107 ++-----------------
 src/lua/trigger.cc                 | 120 ++++++++++++++++++++++
 test/box/bad_trigger.result        |  11 +-
 test/box/bad_trigger.test.py       |   5 +-
 test/box/lua.result                |   4 +-
 test/box/lua_misc.result           |  19 ++--
 test/box/on_replace.result         |  92 +++++++++++++++++
 test/box/on_replace.test.lua       |  37 +++++++
 test/box/session.result            |  74 +++++++-------
 test/box/session.test.lua          |  36 ++++---
 test/box/space.on_replace.result   | 125 -----------------------
 test/box/space.on_replace.test.lua |  51 ----------
 20 files changed, 431 insertions(+), 526 deletions(-)
 create mode 100644 include/lua/trigger.h
 create mode 100644 src/lua/trigger.cc
 create mode 100644 test/box/on_replace.result
 create mode 100644 test/box/on_replace.test.lua
 delete mode 100644 test/box/space.on_replace.result
 delete mode 100644 test/box/space.on_replace.test.lua

diff --git a/include/errcode.h b/include/errcode.h
index be5ae6149b..6235d77b09 100644
--- a/include/errcode.h
+++ b/include/errcode.h
@@ -87,7 +87,7 @@ enum { TNT_ERRMSG_MAX = 512 };
 	/* 32 */_(ER_INDEX_ARITY,		2, "Tuple field count %u is less than required by a defined index (expected %u)") \
 	/* 33 */_(ER_UNUSED33,			2, "Unused33") \
 	/* 34 */_(ER_UNUSED34,			2, "Unused34") \
-	/* 35 */_(ER_UNUSED35,			2, "Unused35") \
+	/* 35 */_(ER_NO_SUCH_TRIGGER,		2, "Trigger is not found") \
 	/* 36 */_(ER_INVALID_MSGPACK,		2, "Invalid MsgPack") \
 	/* 37 */_(ER_TUPLE_NOT_ARRAY,		2, "Tuple/Key must be MsgPack array") \
 	/* 38 */_(ER_KEY_FIELD_TYPE,		2, "Supplied key type of part %u does not match index part type: expected %s") \
diff --git a/include/lua/trigger.h b/include/lua/trigger.h
new file mode 100644
index 0000000000..463e56d467
--- /dev/null
+++ b/include/lua/trigger.h
@@ -0,0 +1,69 @@
+#ifndef INCLUDES_TARANTOOL_LUA_TRIGGER_H
+#define INCLUDES_TARANTOOL_LUA_TRIGGER_H
+/*
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include <trigger.h>
+
+struct lua_State;
+
+/**
+ * Create a Lua trigger, replace an existing one,
+ * or delete a trigger.
+ *
+ * The function accepts a Lua stack with at least
+ * one argument.
+ *
+ * The argument at the top of the stack defines the old value
+ * of the trigger, which serves as a search key if the trigger
+ * needs to be updated. If it is not present or is nil, a new
+ * trigger is created.
+ * The argument just below the top must reference a Lua function
+ * or closure for which the trigger needs to be set. 
+ * If argument below the top is nil, but argument at the top is an
+ * existing trigger, it's erased.
+ *
+ * An existing trigger is searched on the 'list' by checking
+ * trigger->data of all triggers on the list which have the same
+ * trigger->run function as passed in in 'run' argument.
+ *
+ * When a new trigger is set, the function passed in the first
+ * value on Lua stack is referenced, and the reference is saved
+ * in trigger->data (if an old trigger is found it's current Lua
+ * function is first dereferenced, the reference is destroyed).
+ *
+ * @param top defines the top of the stack. If the actual
+ *        lua_gettop(L) is less than 'top', the stack is filled
+ *        with nils (this allows the second argument to be
+ *        optional).
+ */
+int
+lbox_trigger_reset(struct lua_State *L, int top,
+		   struct rlist *list, trigger_f run);
+
+#endif /* INCLUDES_TARANTOOL_LUA_TRIGGER_H */
diff --git a/include/trigger.h b/include/trigger.h
index 49e799f7c6..d3ebeb17e8 100644
--- a/include/trigger.h
+++ b/include/trigger.h
@@ -81,12 +81,4 @@ trigger_clear(struct trigger *trigger)
 	rlist_del_entry(trigger, link);
 }
 
-
-struct lua_trigger
-{
-	struct trigger trigger;
-	int ref;
-};
-
-
 #endif /* INCLUDES_TARANTOOL_TRIGGER_H */
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 35376955c7..8cbbb0b56d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -75,6 +75,7 @@ set (common_sources
      rlist.c
      cpu_feature.c
      lua/fiber.cc
+     lua/trigger.cc
      lua/admin.cc
      lua/info.cc
      lua/stat.cc
diff --git a/src/box/alter.cc b/src/box/alter.cc
index d2a8317902..62402d847f 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -845,7 +845,8 @@ on_drop_space(struct trigger * /* trigger */, void *event)
 	space_delete(space);
 }
 
-static struct trigger drop_space_trigger =  { rlist_nil, on_drop_space, NULL };
+static struct trigger drop_space_trigger =
+	{ rlist_nil, on_drop_space, NULL, NULL };
 
 /**
  * A trigger which is invoked on replace in a data dictionary
@@ -1040,11 +1041,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
 }
 
 struct trigger alter_space_on_replace_space = {
-	rlist_nil, on_replace_dd_space, NULL
+	rlist_nil, on_replace_dd_space, NULL, NULL
 };
 
 struct trigger alter_space_on_replace_index = {
-	rlist_nil, on_replace_dd_index, NULL
+	rlist_nil, on_replace_dd_index, NULL, NULL
 };
 
 /* vim: set foldmethod=marker */
diff --git a/src/box/box_lua.cc b/src/box/box_lua.cc
index 386bb8f3ee..5519c15a2a 100644
--- a/src/box/box_lua.cc
+++ b/src/box/box_lua.cc
@@ -1260,31 +1260,6 @@ box_lua_execute(const struct request *request, struct txn *txn,
 	port_add_lua_multret(port, L);
 }
 
-/**
- * Invoke C function with lua context
- */
-void
-box_luactx(void (*f)(struct lua_State *L, va_list args), ...)
-{
-        lua_State *L = lua_newthread(root_L);
-        int coro_ref = luaL_ref(root_L, LUA_REGISTRYINDEX);
-
-        try {
-                auto scoped_guard = make_scoped_guard([=] {
-                        luaL_unref(root_L, LUA_REGISTRYINDEX, coro_ref);
-                });
-
-                va_list args;
-                va_start(args, f);
-                f(L, args);
-        } catch (const Exception& e) {
-                throw;
-        } catch (...) {
-                tnt_raise(ClientError, ER_PROC_LUA, lua_tostring(L, -1));
-        }
-}
-
-
 static void
 box_index_init_iterator_types(struct lua_State *L, int idx)
 {
diff --git a/src/box/box_lua.h b/src/box/box_lua.h
index d9c01d709f..f3b428f96b 100644
--- a/src/box/box_lua.h
+++ b/src/box/box_lua.h
@@ -43,12 +43,6 @@ box_lua_execute(const struct request *request, struct txn *txn,
 		struct port *port);
 
 
-/**
- * Invoke C function with lua context
- */
-void
-box_luactx(void (*f)(struct lua_State *L, va_list args), ...);
-
 /**
  * Push tuple on lua stack
  */
diff --git a/src/box/box_lua_space.cc b/src/box/box_lua_space.cc
index dee22eb687..46ada056fa 100644
--- a/src/box/box_lua_space.cc
+++ b/src/box/box_lua_space.cc
@@ -28,6 +28,7 @@
  */
 #include "box_lua_space.h"
 #include "lua/utils.h"
+#include "lua/trigger.h"
 
 extern "C" {
 	#include <lua.h>
@@ -43,162 +44,47 @@ extern "C" {
 #include "box_lua.h"
 #include "txn.h"
 
-
-/**
- * Run user trigger with lua context
- */
-static void
-space_user_trigger_luactx(struct lua_State *L, va_list ap)
-{
-	struct lua_trigger *trigger = va_arg(ap, struct lua_trigger *);
-	struct txn *txn = va_arg(ap, struct txn *);
-
-	lua_rawgeti(L, LUA_REGISTRYINDEX, trigger->ref);
-
-	if (txn->old_tuple)
-		lbox_pushtuple(L, txn->old_tuple);
-	else
-		lua_pushnil(L);
-
-	if (txn->new_tuple)
-		lbox_pushtuple(L, txn->new_tuple);
-	else
-		lua_pushnil(L);
-
-	/* TODO: may me space object have to be here */
-	lua_pushstring(L, txn->space->def.name);
-
-	lua_call(L, 3, 0);
-}
-
 /**
  * Trigger function for all spaces
  */
 static void
-space_user_trigger(struct trigger *trigger, void *event)
+lbox_space_on_replace_trigger(struct trigger *trigger, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	box_luactx(space_user_trigger_luactx, trigger, txn);
-}
+	lua_State *L = lua_newthread(tarantool_L);
+	LuarefGuard coro_guard(tarantool_L);
 
-/**
- * lua_trigger destroy method with lua context
- */
-static void
-space_user_trigger_destroy_luaref(struct lua_State *L, va_list ap)
-{
-	int ref = va_arg(ap, int);
-	luaL_unref(L, LUA_REGISTRYINDEX, ref);
-}
+	lua_rawgeti(L, LUA_REGISTRYINDEX, (intptr_t) trigger->data);
 
-/**
- * destroy trigger method (can be called from space_delete method)
- */
-static void
-space_user_trigger_destroy(struct trigger *trigger)
-{
-	struct lua_trigger *lt = (struct lua_trigger *)trigger;
-	trigger_clear(trigger);
-	box_luactx(space_user_trigger_destroy_luaref, lt->ref);
-	free(trigger);
+	lbox_pushtuple(L, txn->old_tuple);
+	lbox_pushtuple(L, txn->new_tuple);
+	/* @todo: maybe the space object has to be here */
+	lua_pushstring(L, txn->space->def.name);
+
+	lbox_call(L, 3, 0);
 }
 
 /**
  * Set/Reset/Get space.on_replace trigger
  */
 static int
-lbox_space_on_replace_trigger(struct lua_State *L)
+lbox_space_on_replace(struct lua_State *L)
 {
 	int top = lua_gettop(L);
 
-	if ( top == 0 || !lua_istable(L, 1))
-		luaL_error(L, "usage: space:on_replace "
-				"instead space.on_replace");
-
-	lua_pushstring(L, "n");
-	lua_rawget(L, 1);
-	if (lua_isnil(L, -1))
-		luaL_error(L, "Can't find space.n property");
-
-	int sno = lua_tointeger(L, -1);
-	lua_pop(L, 1);
-
-
-	struct space *space = space_find(sno);
-
-
-	struct trigger *trigger;
-	struct lua_trigger *current = NULL;
-	rlist_foreach_entry(trigger, &space->on_replace, link) {
-		if (trigger->run == space_user_trigger) {
-			current = (struct lua_trigger *)trigger;
-			break;
-		}
-	}
-
-
-	/* get current trigger function */
-	if (top == 1) {
-		if (!current) {
-			lua_pushnil(L);
-			return 1;
-		}
-		lua_rawgeti(L, LUA_REGISTRYINDEX, current->ref);
-		return 1;
-	}
-
-	/* set or re-set the trigger */
-	if (!lua_isfunction(L, 2) && !lua_isnil(L, 2)) {
+	if (top < 1 || !lua_istable(L, 1)) {
 		luaL_error(L,
-				"usage: space:on_replace([ function | nil ])");
-	}
-
-	/* cleanup trigger */
-	if (lua_isnil(L, 2)) {
-		if (current) {
-			luaL_unref(L, LUA_REGISTRYINDEX, current->ref);
-			trigger_clear(&current->trigger);
-			free(current);
-		}
-		lua_pushnil(L);
-		return 1;
+	   "usage: space:on_replace(function | nil, [function | nil])");
 	}
+	lua_getfield(L, 1, "n"); /* Get space id. */
+	struct space *space = space_find(lua_tointeger(L, lua_gettop(L)));
+	lua_pop(L, 1);
 
-
-
-	/* save ref on trigger function */
-	lua_pushvalue(L, 2);
-	int cb_ref = luaL_ref(L, LUA_REGISTRYINDEX);
-
-	if (current) {
-		luaL_unref(L, LUA_REGISTRYINDEX, current->ref);
-		current->ref = cb_ref;
-		lua_pushvalue(L, 2);
-		return 1;
-	}
-
-	auto scoped_guard = make_scoped_guard([&] {
-		luaL_unref(L, LUA_REGISTRYINDEX, cb_ref);
-	});
-
-
-	current = (struct lua_trigger *)malloc(sizeof(struct lua_trigger));
-
-	if (!current)
-		luaL_error(L, "Can't allocate memory for trigger");
-
-	current->trigger.run = space_user_trigger;
-	current->trigger.destroy = space_user_trigger_destroy;
-	current->ref = cb_ref;
-	trigger_set(&space->on_replace, &current->trigger);
-
-	scoped_guard.is_active = false;
-	lua_pushvalue(L, 2);
-	return 1;
-
+	return lbox_trigger_reset(L, 3,
+				  &space->on_replace,
+				  lbox_space_on_replace_trigger);
 }
 
-
 /**
  * Make a single space available in Lua,
  * via box.space[] array.
@@ -236,7 +122,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
 
         /* space:on_replace */
         lua_pushstring(L, "on_replace");
-        lua_pushcfunction(L, lbox_space_on_replace_trigger);
+        lua_pushcfunction(L, lbox_space_on_replace);
         lua_settable(L, i);
 
 	lua_getfield(L, i, "index");
@@ -323,8 +209,6 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
 	lua_pop(L, 3);	/* cleanup stack - box, schema, space */
 }
 
-
-
 /** Export a space to Lua */
 void
 box_lua_space_new(struct lua_State *L, struct space *space)
diff --git a/src/lua/session.cc b/src/lua/session.cc
index 0887e3f4da..1036ce04a8 100644
--- a/src/lua/session.cc
+++ b/src/lua/session.cc
@@ -28,6 +28,7 @@
  */
 #include "lua/session.h"
 #include "lua/utils.h"
+#include "lua/trigger.h"
 
 extern "C" {
 #include <lua.h>
@@ -38,8 +39,6 @@ extern "C" {
 #include "fiber.h"
 #include "session.h"
 #include "sio.h"
-#include <scoped_guard.h>
-#include "../box/box_lua.h"
 
 static const char *sessionlib_name = "box.session";
 
@@ -92,120 +91,32 @@ lbox_session_peer(struct lua_State *L)
 	return 1;
 }
 
-
-/**
- * run on_connect|on_disconnect trigger with lua context
- */
-static void
-lbox_session_run_trigger_luactx(struct lua_State *L, va_list ap)
-{
-	struct lua_trigger *lt = va_arg(ap, struct lua_trigger *);
-	lua_rawgeti(L, LUA_REGISTRYINDEX, lt->ref);
-	lua_call(L, 0, 0);
-}
-
 /**
  * run on_connect|on_disconnect trigger
  */
 static void
-lbox_session_run_trigger(struct trigger *trg, void * /* event */)
+lbox_session_run_trigger(struct trigger *trigger, void * /* event */)
 {
-	struct lbox_session_trigger *trigger =
-			(struct lbox_session_trigger *) trg;
-	/* Copy the referenced callable object object stack. */
 	lua_State *L = lua_newthread(tarantool_L);
-	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
-	lua_rawgeti(tarantool_L, LUA_REGISTRYINDEX, trigger->ref);
-	/** Move the function to be called to the new coro. */
-	lua_xmove(tarantool_L, L, 1);
-
-	try {
-		lbox_call(L, 0, 0);
-		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);
-	} catch (const Exception& e) {
-		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);
-		throw;
-	}
-}
-
-/**
- * set/reset/get trigger
- */
-static int
-lbox_session_set_trigger(struct lua_State *L, struct rlist *list)
-{
-	int top = lua_gettop(L);
-	if (top > 1 || (top && !lua_isfunction(L, 1) && !lua_isnil(L, 1)))
-		luaL_error(L, "session.on_connect(chunk): bad arguments");
-
-	struct lua_trigger *current = 0;
-	struct trigger *trigger;
-	rlist_foreach_entry(trigger, list, link) {
-		if (trigger->run == lbox_session_run_trigger) {
-			current = (struct lua_trigger *)trigger;
-			break;
-		}
-	}
-
-	/* get current trigger */
-	if (top == 0) {
-		if (current)
-			lua_rawgeti(L, LUA_REGISTRYINDEX, current->ref);
-		else
-			lua_pushnil(L);
-		return 1;
-	}
-
-	/* cleanup the trigger */
-	if (lua_isnil(L, 1)) {
-		if (current) {
-			trigger_clear(&current->trigger);
-			luaL_unref(L, LUA_REGISTRYINDEX, current->ref);
-			free(current);
-		}
-		lua_pushnil(L);
-		return 1;
-	}
-
-	/* set new trigger */
-	lua_pushvalue(L, 1);
-	int ref = luaL_ref(L, LUA_REGISTRYINDEX);
-
-
-	if (current) {
-		luaL_unref(L, LUA_REGISTRYINDEX, current->ref);
-		current->ref = ref;
-	} else {
-		current = (struct lua_trigger *)
-			malloc(sizeof(struct lua_trigger));
-		if (!current) {
-			luaL_unref(L, LUA_REGISTRYINDEX, ref);
-			luaL_error(L, "Can't allocate memory for trigger");
-		}
-		current->trigger.destroy = NULL;
-		current->trigger.run = lbox_session_run_trigger;
-		current->ref = ref;
-		trigger_set(list, &current->trigger);
-	}
-
-	lua_pushvalue(L, 1);
-	return 1;
-
+	LuarefGuard coro_guard(tarantool_L);
+	lua_rawgeti(L, LUA_REGISTRYINDEX, (intptr_t) trigger->data);
+	lbox_call(L, 0, 0);
 }
 
 static int
 lbox_session_on_connect(struct lua_State *L)
 {
-	return lbox_session_set_trigger(L, &session_on_connect);
+	return lbox_trigger_reset(L, 2, &session_on_connect,
+				  lbox_session_run_trigger);
 }
 
 static int
 lbox_session_on_disconnect(struct lua_State *L)
 {
-	return lbox_session_set_trigger(L, &session_on_disconnect);
+	return lbox_trigger_reset(L, 2, &session_on_disconnect,
+				  lbox_session_run_trigger);
 }
 
-
 void
 tarantool_lua_session_init(struct lua_State *L)
 {
diff --git a/src/lua/trigger.cc b/src/lua/trigger.cc
new file mode 100644
index 0000000000..86506156f1
--- /dev/null
+++ b/src/lua/trigger.cc
@@ -0,0 +1,120 @@
+/*
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "lua/trigger.h"
+#include "lua/utils.h"
+
+void
+lbox_trigger_destroy(struct trigger *trigger)
+{
+	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, (intptr_t) trigger->data);
+	free(trigger);
+}
+
+
+struct trigger *
+lbox_trigger_find(struct lua_State *L, int index,
+		  struct rlist *list, trigger_f run)
+{
+	struct trigger *trigger;
+	/** Find the old trigger, if any. */
+	rlist_foreach_entry(trigger, list, link) {
+		if (trigger->run == run) {
+			lua_rawgeti(L, LUA_REGISTRYINDEX,
+				    (intptr_t) trigger->data);
+			bool found = lua_equal(L, index, lua_gettop(L));
+			lua_pop(L, 1);
+			if (found)
+				return trigger;
+		}
+	}
+	return NULL;
+}
+
+void
+lbox_trigger_check_input(struct lua_State *L, int top)
+{
+	assert(lua_checkstack(L, top));
+	/* Push optional arguments. */
+	while (lua_gettop(L) < top)
+		lua_pushnil(L);
+	/* (nil, function) is OK,
+	 * (function, nil), is OK,
+	 * (function, function) is OK,
+	 * anything else is error.
+	 */
+	if ((lua_isfunction(L, top) && lua_isnil(L, top - 1)) ||
+	    (lua_isnil(L, top) && lua_isfunction(L, top - 1)) ||
+	    (lua_isfunction(L, top) && lua_isfunction(L, top - 1)))
+		return;
+
+	luaL_error(L, "trigger reset: incorrect arguments");
+}
+
+int
+lbox_trigger_reset(struct lua_State *L, int top,
+		   struct rlist *list, trigger_f run)
+{
+	lbox_trigger_check_input(L, top);
+
+	struct trigger *trg = lbox_trigger_find(L, top, list, run);
+
+	if (trg) {
+		luaL_unref(L, LUA_REGISTRYINDEX, (intptr_t) trg->data);
+
+	} else if (lua_isfunction(L, top)) {
+		tnt_raise(ClientError, ER_NO_SUCH_TRIGGER);
+	}
+	/*
+	 * During update of a trigger, we must preserve its
+	 * relative position in the list.
+	 */
+	if (lua_isfunction(L, top - 1)) {
+		if (trg == NULL) {
+			trg = (struct trigger *) malloc(sizeof(*trg));
+			if (trg == NULL)
+				luaL_error(L, "failed to allocate trigger");
+			trg->run = run;
+			trg->destroy = lbox_trigger_destroy;
+			trigger_set(list, trg);
+		}
+		/*
+		 * Make the new trigger occupy the top
+		 * slot of the Lua stack.
+		 */
+		lua_pop(L, 1);
+		/* Reference. */
+		trg->data = (void *) (intptr_t)
+			luaL_ref(L, LUA_REGISTRYINDEX);
+
+	} else {
+		trigger_clear(trg);
+		free(trg);
+	}
+	return 0;
+}
diff --git a/test/box/bad_trigger.result b/test/box/bad_trigger.result
index 362ef175bc..a28855fbba 100644
--- a/test/box/bad_trigger.result
+++ b/test/box/bad_trigger.result
@@ -3,19 +3,20 @@
  # if on_connect() trigger raises an exception, the connection is dropped
  # 
  
-type(box.session.on_connect(function() nosuchfunction() end))
+function f1() nosuchfunction() end
+---
+...
+box.session.on_connect(f1)
 ---
-- function
 ...
 select * from t0 where k0=0
 ---
 - error:
     errcode: ER_PROC_LUA
-    errmsg: [string "return type(box.session.on_connect(function()..."]:1: attempt to call global 'nosuchfunction' (a nil value)
+    errmsg: [string "function f1() nosuchfunction() end"]:1: attempt to call global 'nosuchfunction' (a nil value)
 ...
 Connection is dead.
 
-type(box.session.on_connect(nil))
+box.session.on_connect(nil, f1)
 ---
-- nil
 ...
diff --git a/test/box/bad_trigger.test.py b/test/box/bad_trigger.test.py
index d41473330a..547ab82237 100644
--- a/test/box/bad_trigger.test.py
+++ b/test/box/bad_trigger.test.py
@@ -5,7 +5,8 @@ print """
  # 
  """
 
-admin("type(box.session.on_connect(function() nosuchfunction() end))")
+admin("function f1() nosuchfunction() end")
+admin("box.session.on_connect(f1)")
 con1 = BoxConnection('localhost', server.primary_port)
 con1("select * from t0 where k0=0")
 if not con1.check_connection():
@@ -13,4 +14,4 @@ if not con1.check_connection():
 else:
     print "Connection is alive.\n"
 # Clean-up
-admin("type(box.session.on_connect(nil))")
+admin("box.session.on_connect(nil, f1)")
diff --git a/test/box/lua.result b/test/box/lua.result
index 22fccbfe5e..14d952043c 100644
--- a/test/box/lua.result
+++ b/test/box/lua.result
@@ -498,7 +498,7 @@ t
   - 'backlog: 1024'
   - 'wal_dir_rescan_delay: 0.1'
 ...
-t = {} for k,v in pairs(box.space[0]) do if type(v) ~= 'table' then table.insert(t, k..': '..tostring(v)) end end
+t = {} for k,v in pairs(box.space[0]) do if type(v) ~= 'table' and type(v) ~= 'function' then table.insert(t, k..': '..tostring(v)) end end
 ---
 ...
 t
@@ -549,7 +549,7 @@ t
   - 'backlog: 1024'
   - 'wal_dir_rescan_delay: 0.1'
 ...
-t = {} for k,v in pairs(box.space[0]) do if type(v) ~= 'table' then table.insert(t, k..': '..tostring(v)) end end
+t = {} for k,v in pairs(box.space[0]) do if type(v) ~= 'table' and type(v) ~= 'function' then table.insert(t, k..': '..tostring(v)) end end
 ---
 ...
 t
diff --git a/test/box/lua_misc.result b/test/box/lua_misc.result
index 1a4a244f2c..6a8a39d5d1 100644
--- a/test/box/lua_misc.result
+++ b/test/box/lua_misc.result
@@ -136,7 +136,7 @@ t;
   - 'box.error.ER_INDEX_ARITY : 8194'
   - 'box.error.ER_WAL_IO : 9986'
   - 'box.error.ER_INJECTION : 2306'
-  - 'box.error.ER_LAST_DROP : 7426'
+  - 'box.error.ER_DROP_PRIMARY_KEY : 7682'
   - 'box.error.ER_INDEX_TYPE : 1282'
   - 'box.error.ER_ARG_TYPE : 10498'
   - 'box.error.ER_INVALID_MSGPACK : 9218'
@@ -145,29 +145,30 @@ t;
   - 'box.error.ER_ILLEGAL_PARAMS : 514'
   - 'box.error.ER_KEY_FIELD_TYPE : 9730'
   - 'box.error.ER_NONMASTER : 258'
-  - 'box.error.ER_TUPLE_IS_TOO_LONG : 11010'
+  - 'box.error.ER_FIELD_TYPE_MISMATCH : 11778'
   - 'box.error.ER_MODIFY_INDEX : 6914'
   - 'box.error.ER_EXACT_MATCH : 11522'
   - 'box.error.ER_SECONDARY : 770'
   - 'box.error.ER_NO_SUCH_SPACE : 14594'
+  - 'box.error.ER_TUPLE_FOUND : 14082'
   - 'box.error.ER_UPDATE_FIELD : 14338'
   - 'box.error.ER_DROP_SPACE : 6146'
   - 'box.error.ER_UNKNOWN_UPDATE_OP : 11266'
-  - 'box.error.ER_TUPLE_FOUND : 14082'
-  - 'box.error.ER_UNSUPPORTED : 2562'
   - 'box.error.ER_NO_SUCH_FIELD : 13826'
-  - 'box.error.ER_TUPLE_NOT_ARRAY : 9474'
+  - 'box.error.ER_UNSUPPORTED : 2562'
   - 'box.error.ER_SPACE_DISABLED : 13314'
+  - 'box.error.ER_TUPLE_NOT_ARRAY : 9474'
   - 'box.error.ER_PROC_LUA : 13058'
-  - 'box.error.ER_ALTER_SPACE : 6402'
   - 'box.error.ER_FIBER_STACK : 6658'
+  - 'box.error.ER_ALTER_SPACE : 6402'
   - 'box.error.ER_TUPLE_IS_RO : 1025'
+  - 'box.error.ER_LAST_DROP : 7426'
   - 'box.error.ER_NO_SUCH_PROC : 12802'
-  - 'box.error.ER_DROP_PRIMARY_KEY : 7682'
+  - 'box.error.ER_SPACE_EXISTS : 1538'
+  - 'box.error.ER_NO_SUCH_TRIGGER : 8962'
   - 'box.error.ER_SPACE_ARITY : 7938'
   - 'box.error.ER_SPLICE : 10754'
-  - 'box.error.ER_FIELD_TYPE_MISMATCH : 11778'
-  - 'box.error.ER_SPACE_EXISTS : 1538'
+  - 'box.error.ER_TUPLE_IS_TOO_LONG : 11010'
 ...
 space:drop();
 ---
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
new file mode 100644
index 0000000000..ae5c0205ca
--- /dev/null
+++ b/test/box/on_replace.result
@@ -0,0 +1,92 @@
+ts = box.schema.create_space('test_space')
+---
+...
+ts:create_index('primary', 'hash')
+---
+...
+type(ts.on_replace)
+---
+- function
+...
+ts.on_replace()
+---
+- error: 'usage: space:on_replace(function | nil, [function | nil])'
+...
+ts:on_replace()
+---
+- error: 'trigger reset: incorrect arguments'
+...
+ts:on_replace(123)
+---
+- error: 'trigger reset: incorrect arguments'
+...
+function fail(old_tuple, new_tuple) error('test') end
+---
+...
+ts:on_replace(fail)
+---
+...
+ts:insert(1, 'b', 'c')
+---
+- error: '[string "function fail(old_tuple, new_tuple) error(''te..."]:1: test'
+...
+ts:select(0, 1)
+---
+...
+ts:on_replace(nil, fail)
+---
+...
+ts:insert(1, 'b', 'c')
+---
+- [1, 'b', 'c']
+...
+ts:select(0, 1)
+---
+- [1, 'b', 'c']
+...
+function fail(old_tuple, new_tuple) error('abc') end
+---
+...
+ts:on_replace(fail)
+---
+...
+ts:insert(2, 'b', 'c')
+---
+- error: '[string "function fail(old_tuple, new_tuple) error(''ab..."]:1: abc'
+...
+ts:select(0, 2)
+---
+...
+function save_out(told, tnew) o = told n = tnew end
+---
+...
+ts:on_replace(save_out, fail)
+---
+...
+ts:insert(2, 'a', 'b', 'c')
+---
+- [2, 'a', 'b', 'c']
+...
+o
+---
+- null
+...
+n
+---
+- [2, 'a', 'b', 'c']
+...
+ts:replace(2, 'd', 'e', 'f')
+---
+- [2, 'd', 'e', 'f']
+...
+o
+---
+- [2, 'a', 'b', 'c']
+...
+n
+---
+- [2, 'd', 'e', 'f']
+...
+ts:drop()
+---
+...
diff --git a/test/box/on_replace.test.lua b/test/box/on_replace.test.lua
new file mode 100644
index 0000000000..6d15735e6d
--- /dev/null
+++ b/test/box/on_replace.test.lua
@@ -0,0 +1,37 @@
+ts = box.schema.create_space('test_space')
+ts:create_index('primary', 'hash')
+
+type(ts.on_replace)
+ts.on_replace()
+ts:on_replace()
+ts:on_replace(123)
+
+function fail(old_tuple, new_tuple) error('test') end
+ts:on_replace(fail)
+
+ts:insert(1, 'b', 'c')
+ts:select(0, 1)
+
+ts:on_replace(nil, fail)
+
+ts:insert(1, 'b', 'c')
+ts:select(0, 1)
+
+function fail(old_tuple, new_tuple) error('abc') end
+ts:on_replace(fail)
+
+ts:insert(2, 'b', 'c')
+ts:select(0, 2)
+
+function save_out(told, tnew) o = told n = tnew end
+ts:on_replace(save_out, fail)
+
+ts:insert(2, 'a', 'b', 'c')
+o
+n
+
+ts:replace(2, 'd', 'e', 'f')
+o
+n
+
+ts:drop()
diff --git a/test/box/session.result b/test/box/session.result
index 37eaf1e6a3..d567dca5dd 100644
--- a/test/box/session.result
+++ b/test/box/session.result
@@ -51,64 +51,64 @@ box.session.peer() == box.session.peer(box.session.id())
 - true
 ...
 -- check on_connect/on_disconnect triggers
-type(box.session.on_connect(function() end))
+function noop() end
 ---
-- function
 ...
-type(box.session.on_disconnect(function() end))
+box.session.on_connect(noop)
+---
+...
+box.session.on_disconnect(noop)
 ---
-- function
 ...
 -- check it's possible to reset these triggers
-type(box.session.on_connect(function() error('hear') end))
+function fail() error('hear') end
+---
+...
+box.session.on_connect(fail, noop)
 ---
-- function
 ...
-type(box.session.on_disconnect(function() error('hear') end))
+box.session.on_disconnect(fail, noop)
 ---
-- function
 ...
 -- check on_connect/on_disconnect argument count and type
-type(box.session.on_connect())
+box.session.on_connect()
 ---
-- function
+- error: 'trigger reset: incorrect arguments'
 ...
-type(box.session.on_disconnect())
+box.session.on_disconnect()
 ---
-- function
+- error: 'trigger reset: incorrect arguments'
 ...
 box.session.on_connect(function() end, function() end)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: Trigger is not found
 ...
 box.session.on_disconnect(function() end, function() end)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: Trigger is not found
 ...
 box.session.on_connect(1, 2)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: 'trigger reset: incorrect arguments'
 ...
 box.session.on_disconnect(1, 2)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: 'trigger reset: incorrect arguments'
 ...
 box.session.on_connect(1)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: 'trigger reset: incorrect arguments'
 ...
 box.session.on_disconnect(1)
 ---
-- error: 'session.on_connect(chunk): bad arguments'
+- error: 'trigger reset: incorrect arguments'
 ...
 -- use of nil to clear the trigger
-box.session.on_connect(nil)
+box.session.on_connect(nil, fail)
 ---
-- null
 ...
-box.session.on_disconnect(nil)
+box.session.on_disconnect(nil, fail)
 ---
-- null
 ...
 -- check how connect/disconnect triggers work
 function inc() active_connections = active_connections + 1 end
@@ -117,13 +117,11 @@ function inc() active_connections = active_connections + 1 end
 function dec() active_connections = active_connections - 1 end
 ---
 ...
-type(box.session.on_connect(inc))
+box.session.on_connect(inc)
 ---
-- function
 ...
-type(box.session.on_disconnect(dec))
+box.session.on_disconnect(dec)
 ---
-- function
 ...
 active_connections = 0
 ---
@@ -147,22 +145,24 @@ active_connections
 ---
 - 0
 ...
-box.session.on_connect(nil)
+box.session.on_connect(nil, inc)
 ---
-- null
 ...
-box.session.on_disconnect(nil)
+box.session.on_disconnect(nil, dec)
 ---
-- null
 ...
 -- write audit trail of connect/disconnect into a space
-type(box.session.on_connect(function() box.space['tweedledum']:insert(box.session.id()) end))
+function audit_connect() box.space['tweedledum']:insert(box.session.id()) end
+---
+...
+function audit_disconnect() box.space['tweedledum']:delete(box.session.id()) end
+---
+...
+box.session.on_connect(audit_connect)
 ---
-- function
 ...
-type(box.session.on_disconnect(function() box.space['tweedledum']:delete(box.session.id()) end))
+box.session.on_disconnect(audit_disconnect)
 ---
-- function
 ...
 --# create connection con_three to default
 --# set connection con_three
@@ -173,13 +173,11 @@ space:select(0, box.session.id())[0] == box.session.id()
 --# set connection default
 --# drop connection con_three
 -- cleanup
-box.session.on_connect(nil)
+box.session.on_connect(nil, audit_connect)
 ---
-- null
 ...
-box.session.on_disconnect(nil)
+box.session.on_disconnect(nil, audit_disconnect)
 ---
-- null
 ...
 active_connections
 ---
diff --git a/test/box/session.test.lua b/test/box/session.test.lua
index c3ff396c73..1887b599b0 100644
--- a/test/box/session.test.lua
+++ b/test/box/session.test.lua
@@ -17,16 +17,18 @@ failed
 box.session.peer() == box.session.peer(box.session.id())
 
 -- check on_connect/on_disconnect triggers
-type(box.session.on_connect(function() end))
-type(box.session.on_disconnect(function() end))
+function noop() end
+box.session.on_connect(noop)
+box.session.on_disconnect(noop)
 
 -- check it's possible to reset these triggers
-type(box.session.on_connect(function() error('hear') end))
-type(box.session.on_disconnect(function() error('hear') end))
+function fail() error('hear') end
+box.session.on_connect(fail, noop)
+box.session.on_disconnect(fail, noop)
 
 -- check on_connect/on_disconnect argument count and type
-type(box.session.on_connect())
-type(box.session.on_disconnect())
+box.session.on_connect()
+box.session.on_disconnect()
 
 box.session.on_connect(function() end, function() end)
 box.session.on_disconnect(function() end, function() end)
@@ -38,14 +40,14 @@ box.session.on_connect(1)
 box.session.on_disconnect(1)
 
 -- use of nil to clear the trigger
-box.session.on_connect(nil)
-box.session.on_disconnect(nil)
+box.session.on_connect(nil, fail)
+box.session.on_disconnect(nil, fail)
 
 -- check how connect/disconnect triggers work
 function inc() active_connections = active_connections + 1 end
 function dec() active_connections = active_connections - 1 end
-type(box.session.on_connect(inc))
-type(box.session.on_disconnect(dec))
+box.session.on_connect(inc)
+box.session.on_disconnect(dec)
 active_connections = 0
 --# create connection con_one to default
 active_connections
@@ -56,12 +58,14 @@ active_connections
 box.fiber.sleep(0) -- yield
 active_connections
 
-box.session.on_connect(nil)
-box.session.on_disconnect(nil)
+box.session.on_connect(nil, inc)
+box.session.on_disconnect(nil, dec)
 
 -- write audit trail of connect/disconnect into a space
-type(box.session.on_connect(function() box.space['tweedledum']:insert(box.session.id()) end))
-type(box.session.on_disconnect(function() box.space['tweedledum']:delete(box.session.id()) end))
+function audit_connect() box.space['tweedledum']:insert(box.session.id()) end
+function audit_disconnect() box.space['tweedledum']:delete(box.session.id()) end
+box.session.on_connect(audit_connect)
+box.session.on_disconnect(audit_disconnect)
 
 --# create connection con_three to default
 --# set connection con_three
@@ -70,8 +74,8 @@ space:select(0, box.session.id())[0] == box.session.id()
 --# drop connection con_three
 
 -- cleanup
-box.session.on_connect(nil)
-box.session.on_disconnect(nil)
+box.session.on_connect(nil, audit_connect)
+box.session.on_disconnect(nil, audit_disconnect)
 active_connections
 
 space:drop()
diff --git a/test/box/space.on_replace.result b/test/box/space.on_replace.result
deleted file mode 100644
index c83871a21d..0000000000
--- a/test/box/space.on_replace.result
+++ /dev/null
@@ -1,125 +0,0 @@
-ts = box.schema.create_space('ttest_space')
----
-...
-ts:create_index('primary', 'hash')
----
-...
-type(ts.on_replace)
----
-- function
-...
-ts.on_replace()
----
-- error: 'usage: space:on_replace instead space.on_replace'
-...
-ts:on_replace()
----
-- null
-...
-ts:on_replace(123)
----
-- error: 'usage: space:on_replace([ function | nil ])'
-...
-type(ts:on_replace(function(old_tuple, new_tuple) error('test') end))
----
-- function
-...
-type(ts:on_replace())
----
-- function
-...
-ts:on_replace()()
----
-- error: '[string "return type(ts:on_replace(function(old_tuple,..."]:1: test'
-...
-ts:insert(1, 'b', 'c')
----
-- error: 'Lua error: [string "return type(ts:on_replace(function(old_tuple,..."]:1:
-    test'
-...
-ts:select(0, 1)
----
-...
-ts:on_replace(nil)
----
-- null
-...
-ts:insert(1, 'b', 'c')
----
-- [1, 'b', 'c']
-...
-ts:select(0, 1)
----
-- [1, 'b', 'c']
-...
-type(ts:on_replace(function(old_tuple, new_tuple) error('abc') end))
----
-- function
-...
-ts:insert(2, 'b', 'c')
----
-- error: 'Lua error: [string "return type(ts:on_replace(function(old_tuple,..."]:1:
-    abc'
-...
-ts:select(0, 2)
----
-...
-type(ts:on_replace(function(told, tnew) o = told n = tnew end))
----
-- function
-...
-ts:insert(2, 'a', 'b', 'c')
----
-- [2, 'a', 'b', 'c']
-...
-o == nil
----
-- true
-...
-n ~= nil
----
-- true
-...
-n[1] == 'a'
----
-- true
-...
-n[2] == 'b'
----
-- true
-...
-n[3] == 'c'
----
-- true
-...
-ts:replace(2, 'b', 'c', 'd')
----
-- [2, 'b', 'c', 'd']
-...
-o ~= nil
----
-- true
-...
-n ~= nil
----
-- true
-...
-o[1] == 'a'
----
-- true
-...
-n[1] == 'b'
----
-- true
-...
-o[2] == 'b'
----
-- true
-...
-n[2] == 'c'
----
-- true
-...
-ts:drop()
----
-...
diff --git a/test/box/space.on_replace.test.lua b/test/box/space.on_replace.test.lua
deleted file mode 100644
index 81b218ba15..0000000000
--- a/test/box/space.on_replace.test.lua
+++ /dev/null
@@ -1,51 +0,0 @@
-ts = box.schema.create_space('ttest_space')
-ts:create_index('primary', 'hash')
-
-type(ts.on_replace)
-ts.on_replace()
-ts:on_replace()
-ts:on_replace(123)
-
-type(ts:on_replace(function(old_tuple, new_tuple) error('test') end))
-
-
-type(ts:on_replace())
-ts:on_replace()()
-
-ts:insert(1, 'b', 'c')
-ts:select(0, 1)
-
-
-ts:on_replace(nil)
-
-ts:insert(1, 'b', 'c')
-ts:select(0, 1)
-
-
-type(ts:on_replace(function(old_tuple, new_tuple) error('abc') end))
-
-
-ts:insert(2, 'b', 'c')
-ts:select(0, 2)
-
-
-
-type(ts:on_replace(function(told, tnew) o = told n = tnew end))
-
-ts:insert(2, 'a', 'b', 'c')
-o == nil
-n ~= nil
-n[1] == 'a'
-n[2] == 'b'
-n[3] == 'c'
-
-ts:replace(2, 'b', 'c', 'd')
-o ~= nil
-n ~= nil
-o[1] == 'a'
-n[1] == 'b'
-o[2] == 'b'
-n[2] == 'c'
-
-
-ts:drop()
-- 
GitLab