From 8d12596690e40e8c83d153647615c655e65f75ff Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Thu, 25 Sep 2014 17:02:24 +0400
Subject: [PATCH] Fix a memory leak at sberlabs.

The wal writer now allocates memory to encode a binary log row,
but never used to free it. Add fiber_gc().

Restore fiber_gc() in recovery, and also move index allocation
from runtime to memtx arena.

Minor fixes & cleanups.
---
 src/box/box.cc         |  3 ++-
 src/box/iproto.cc      |  1 -
 src/box/lua/slab.cc    | 10 +++++-----
 src/box/recovery.cc    |  6 +-----
 src/box/recovery.h     |  2 --
 src/box/replication.cc | 10 +++++-----
 src/box/request.cc     |  4 ++++
 src/box/tree_index.cc  |  2 +-
 src/box/tuple.cc       | 24 ++++++++++++------------
 src/box/tuple.h        |  5 +++--
 src/box/txn.cc         | 21 ++++++++++++++-------
 src/box/txn.h          | 10 ++--------
 src/util.cc            |  2 +-
 test/wal/oom.result    | 12 ++++++------
 14 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index cc89677d19..5e87fd31fc 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -422,6 +422,7 @@ box_init()
 		space_end_recover_snapshot();
 		snapshot_save(recovery);
 	}
+	fiber_gc();
 
 	title("orphan", NULL);
 	recovery_follow_local(recovery,
@@ -519,7 +520,7 @@ box_snapshot(void)
 		return (WIFSIGNALED(status) ? EINTR : WEXITSTATUS(status));
 	}
 
-	slab_arena_mprotect(&tuple_arena);
+	slab_arena_mprotect(&memtx_arena);
 
 	cord_set_name("snap");
 	title("dumper", "%" PRIu32, getppid());
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index ca8b68d271..512d11f0b4 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -490,7 +490,6 @@ iproto_enqueue_batch(struct iproto_connection *con, struct ibuf *in)
 		xrow_header_decode(&ireq->header, &pos, reqend);
 		ireq->total_len = pos - reqstart; /* total request length */
 
-
 		/*
 		 * sic: in case of exception con->parse_size
 		 * as well as in->pos must not be advanced, to
diff --git a/src/box/lua/slab.cc b/src/box/lua/slab.cc
index d0abaaf44a..400c53d957 100644
--- a/src/box/lua/slab.cc
+++ b/src/box/lua/slab.cc
@@ -89,7 +89,7 @@ lbox_slab_info(struct lua_State *L)
 	lua_pushstring(L, "slabs");
 	lua_newtable(L);
 	luaL_setmaphint(L, -1);
-	small_stats(&talloc, &totals, small_stats_lua_cb, L);
+	small_stats(&memtx_alloc, &totals, small_stats_lua_cb, L);
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "arena_used");
@@ -103,15 +103,15 @@ lbox_slab_info(struct lua_State *L)
 	char value[32];
 	double items_used_ratio = 100
 		* ((double)totals.used)
-		/ ((double)talloc.cache->arena->prealloc + 0.0001);
+		/ ((double)memtx_alloc.cache->arena->prealloc + 0.0001);
 	snprintf(value, sizeof(value), "%0.1lf%%", items_used_ratio);
 	lua_pushstring(L, "items_used_ratio");
 	lua_pushstring(L, value);
 	lua_settable(L, -3);
 
 	double arena_used_ratio = 100
-		* ((double)talloc.cache->arena->used)
-		/ ((double)talloc.cache->arena->prealloc + 0.0001);
+		* ((double)memtx_alloc.cache->arena->used)
+		/ ((double)memtx_alloc.cache->arena->prealloc + 0.0001);
 	snprintf(value, sizeof(value), "%0.1lf%%", arena_used_ratio);
 	lua_pushstring(L, "arena_used_ratio");
 	lua_pushstring(L, value);
@@ -139,7 +139,7 @@ lbox_runtime_info(struct lua_State *L)
 static int
 lbox_slab_check(struct lua_State *L __attribute__((unused)))
 {
-	slab_cache_check(talloc.cache);
+	slab_cache_check(memtx_alloc.cache);
 	return 0;
 }
 
diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index e022be3108..6ce57146fc 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -187,7 +187,6 @@ recovery_init(const char *snap_dirname, const char *wal_dirname,
 		panic("can't scan snapshot directory");
 	if (log_dir_scan(&r->wal_dir) != 0)
 		panic("can't scan WAL directory");
-	region_create(&r->pool, &cord()->slabc);
 }
 
 void
@@ -227,8 +226,6 @@ recovery_free()
 		 */
 		log_io_close(&r->current_wal);
 	}
-
-	region_destroy(&r->pool);
 	recovery= NULL;
 }
 
@@ -485,9 +482,7 @@ recover_remaining_wals(struct recovery_state *r)
 		result = -1;
 	}
 
-#if 0
 	region_free(&fiber()->gc);
-#endif
 	return result;
 }
 
@@ -1028,6 +1023,7 @@ wal_write_to_disk(struct recovery_state *r, struct wal_writer *writer,
 			break;
 		req = write_end;
 	}
+	fiber_gc();
 	STAILQ_SPLICE(input, write_end, wal_fifo_entry, rollback);
 	STAILQ_CONCAT(commit, input);
 }
diff --git a/src/box/recovery.h b/src/box/recovery.h
index 2d6e2eba04..60fff69fa0 100644
--- a/src/box/recovery.h
+++ b/src/box/recovery.h
@@ -89,8 +89,6 @@ struct recovery_state {
 	uint32_t server_id;
 
 	bool finalize;
-	/** Throwaway memory for spot allocations. */
-	struct region pool;
 };
 
 extern struct recovery_state *recovery;
diff --git a/src/box/replication.cc b/src/box/replication.cc
index e9d6a4c450..295c133514 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -213,11 +213,11 @@ struct replication_request {
 
 /** Replication acceptor fiber handler. */
 void
-replication_join(int fd, struct xrow_header *header)
+replication_join(int fd, struct xrow_header *packet)
 {
-	assert(header->type == IPROTO_JOIN);
+	assert(packet->type == IPROTO_JOIN);
 	struct tt_uuid server_uuid = uuid_nil;
-	xrow_decode_join(header, &server_uuid);
+	xrow_decode_join(packet, &server_uuid);
 
 	/* Notify box about new cluster server */
 	recovery->join_handler(&server_uuid);
@@ -230,8 +230,8 @@ replication_join(int fd, struct xrow_header *header)
 	}
 	request->fd = fd;
 	request->io.data = request;
-	request->data.type = header->type;
-	request->data.sync = header->sync;
+	request->data.type = packet->type;
+	request->data.sync = packet->sync;
 
 	ev_io_init(&request->io, replication_send_socket,
 		   master_to_spawner_socket, EV_WRITE);
diff --git a/src/box/request.cc b/src/box/request.cc
index 200c649396..f8a19d2aa5 100644
--- a/src/box/request.cc
+++ b/src/box/request.cc
@@ -155,6 +155,10 @@ execute_select(struct request *request, struct port *port)
 			break;
 		port_add_tuple(port, tuple);
 	}
+	if (! in_txn()) {
+		 /* no txn is created, so simply collect garbage here */
+		fiber_gc();
+	}
 }
 
 void
diff --git a/src/box/tree_index.cc b/src/box/tree_index.cc
index cc45bdf8ba..6e762e7d45 100644
--- a/src/box/tree_index.cc
+++ b/src/box/tree_index.cc
@@ -204,7 +204,7 @@ TreeIndex::TreeIndex(struct key_def *key_def_arg)
 	  build_array_alloc_size(0)
 {
 	if (tree_extent_pool_initialized == 0) {
-		mempool_create(&tree_extent_pool, &cord()->slabc,
+		mempool_create(&tree_extent_pool, &memtx_slab_cache,
 			       BPS_TREE_EXTENT_SIZE);
 		tree_extent_pool_initialized = 1;
 	}
diff --git a/src/box/tuple.cc b/src/box/tuple.cc
index 383304acec..69d2dff189 100644
--- a/src/box/tuple.cc
+++ b/src/box/tuple.cc
@@ -45,9 +45,9 @@ static uint32_t formats_size, formats_capacity;
 
 uint32_t snapshot_version;
 
-struct slab_arena tuple_arena;
-static struct slab_cache tuple_slab_cache;
-struct small_alloc talloc;
+struct slab_arena memtx_arena;
+struct slab_cache memtx_slab_cache;
+struct small_alloc memtx_alloc;
 
 /** Extract all available type info from keys. */
 void
@@ -252,7 +252,7 @@ struct tuple *
 tuple_alloc(struct tuple_format *format, size_t size)
 {
 	size_t total = sizeof(struct tuple) + size + format->field_map_size;
-	char *ptr = (char *) smalloc(&talloc, total, "tuple");
+	char *ptr = (char *) smalloc(&memtx_alloc, total, "tuple");
 	struct tuple *tuple = (struct tuple *)(ptr + format->field_map_size);
 
 	tuple->refs = 0;
@@ -277,10 +277,10 @@ tuple_delete(struct tuple *tuple)
 	struct tuple_format *format = tuple_format(tuple);
 	char *ptr = (char *) tuple - format->field_map_size;
 	tuple_format_ref(format, -1);
-	if (!talloc.is_delayed_free_mode || tuple->version == snapshot_version)
-		smfree(&talloc, ptr);
+	if (!memtx_alloc.is_delayed_free_mode || tuple->version == snapshot_version)
+		smfree(&memtx_alloc, ptr);
 	else
-		smfree_delayed(&talloc, ptr);
+		smfree_delayed(&memtx_alloc, ptr);
 }
 
 /**
@@ -545,14 +545,14 @@ tuple_init(float arena_prealloc, uint32_t objsize_min,
 		flags = MAP_SHARED;
 	}
 
-	if (slab_arena_create(&tuple_arena, prealloc, prealloc,
+	if (slab_arena_create(&memtx_arena, prealloc, prealloc,
 			      slab_size, flags)) {
 		panic_syserror("failed to preallocate %zu bytes",
 			       prealloc);
 	}
-	slab_cache_create(&tuple_slab_cache, &tuple_arena,
+	slab_cache_create(&memtx_slab_cache, &memtx_arena,
 			  slab_size);
-	small_alloc_create(&talloc, &tuple_slab_cache,
+	small_alloc_create(&memtx_alloc, &memtx_slab_cache,
 			   objsize_min, alloc_factor);
 }
 
@@ -577,11 +577,11 @@ void
 tuple_begin_snapshot()
 {
 	snapshot_version++;
-	small_alloc_setopt(&talloc, SMALL_DELAYED_FREE_MODE, true);
+	small_alloc_setopt(&memtx_alloc, SMALL_DELAYED_FREE_MODE, true);
 }
 
 void
 tuple_end_snapshot()
 {
-	small_alloc_setopt(&talloc, SMALL_DELAYED_FREE_MODE, false);
+	small_alloc_setopt(&memtx_alloc, SMALL_DELAYED_FREE_MODE, false);
 }
diff --git a/src/box/tuple.h b/src/box/tuple.h
index b07a3e4db7..6e79795fee 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -35,8 +35,9 @@ enum { FORMAT_ID_MAX = UINT16_MAX - 1, FORMAT_ID_NIL = UINT16_MAX };
 
 struct tbuf;
 
-extern struct small_alloc talloc;
-extern struct slab_arena tuple_arena;
+extern struct small_alloc memtx_alloc;
+extern struct slab_cache memtx_slab_cache;
+extern struct slab_arena memtx_arena;
 
 /**
  * @brief In-memory tuple format
diff --git a/src/box/txn.cc b/src/box/txn.cc
index 7f32f24ee3..c350593c9d 100644
--- a/src/box/txn.cc
+++ b/src/box/txn.cc
@@ -40,6 +40,13 @@
 
 double too_long_threshold;
 
+static inline void
+fiber_set_txn(struct fiber *fiber, struct txn *txn)
+{
+	fiber_set_key(fiber, FIBER_KEY_TXN, (void *) txn);
+}
+
+
 static void
 txn_add_redo(struct txn_stmt *stmt, struct request *request)
 {
@@ -132,7 +139,7 @@ txn_stmt_new(struct txn *txn)
 struct txn *
 txn_begin(bool autocommit)
 {
-	assert(! fiber_get_txn(fiber()));
+	assert(! in_txn());
 	struct txn *txn = (struct txn *)
 		region_alloc0(&fiber()->gc, sizeof(*txn));
 	rlist_create(&txn->stmts);
@@ -153,7 +160,7 @@ txn_begin(bool autocommit)
 struct txn *
 txn_begin_stmt(struct request *request)
 {
-	struct txn *txn = fiber_get_txn(fiber());
+	struct txn *txn = in_txn();
 	if (txn == NULL)
 		txn = txn_begin(true);
 	struct txn_stmt *stmt = txn_stmt_new(txn);
@@ -182,7 +189,7 @@ txn_commit_stmt(struct txn *txn, struct port *port)
 void
 txn_commit(struct txn *txn)
 {
-	assert(txn == fiber_get_txn(fiber()));
+	assert(txn == in_txn());
 	struct txn_stmt *stmt;
 	/* if (!txn->autocommit && txn->n_stmts && engine_no_yield(txn->engine)) */
 		trigger_clear(&txn->fiber_on_yield);
@@ -237,7 +244,7 @@ txn_finish(struct txn *txn)
 void
 txn_rollback_stmt()
 {
-	struct txn *txn = fiber_get_txn(fiber());
+	struct txn *txn = in_txn();
 	if (txn == NULL)
 		return;
 	if (txn->autocommit)
@@ -257,7 +264,7 @@ txn_rollback_stmt()
 void
 txn_rollback()
 {
-	struct txn *txn = fiber_get_txn(fiber());
+	struct txn *txn = in_txn();
 	if (txn == NULL)
 		return;
 	struct txn_stmt *stmt;
@@ -296,7 +303,7 @@ int
 boxffi_txn_begin()
 {
 	try {
-		if (fiber_get_txn(fiber()))
+		if (in_txn())
 			tnt_raise(ClientError, ER_ACTIVE_TRANSACTION);
 		(void) txn_begin(false);
 	} catch (Exception  *e) {
@@ -309,7 +316,7 @@ int
 boxffi_txn_commit()
 {
 	try {
-		struct txn *txn = fiber_get_txn(fiber());
+		struct txn *txn = in_txn();
 		/**
 		 * COMMIT is like BEGIN or ROLLBACK
 		 * a "transaction-initiating statement".
diff --git a/src/box/txn.h b/src/box/txn.h
index bcf03e6459..8f8247c931 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -75,15 +75,9 @@ struct txn {
 
 /* Pointer to the current transaction (if any) */
 static inline struct txn *
-fiber_get_txn(struct fiber *fiber)
+in_txn()
 {
-	return (struct txn *) fiber_get_key(fiber, FIBER_KEY_TXN);
-}
-
-static inline void
-fiber_set_txn(struct fiber *fiber, struct txn *txn)
-{
-	fiber_set_key(fiber, FIBER_KEY_TXN, (void *) txn);
+	return (struct txn *) fiber_get_key(fiber(), FIBER_KEY_TXN);
 }
 
 /**
diff --git a/src/util.cc b/src/util.cc
index 6f30246714..f0cd3ca37c 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -314,7 +314,7 @@ backtrace_foreach(backtrace_cb cb, void *frame_, void *stack, size_t stack_size,
 	}
 }
 
-void
+extern "C" void
 print_backtrace()
 {
 	void *frame = __builtin_frame_address(0);
diff --git a/test/wal/oom.result b/test/wal/oom.result
index 089a7c863c..1d14c34789 100644
--- a/test/wal/oom.result
+++ b/test/wal/oom.result
@@ -15,11 +15,11 @@ while true do
     i = i + 1
 end;
 ---
-- error: Failed to allocate 268 bytes in slab allocator for tuple
+- error: Failed to allocate 260 bytes in slab allocator for tuple
 ...
 space:len();
 ---
-- 62
+- 60
 ...
 i = 1;
 ---
@@ -29,11 +29,11 @@ while true do
     i = i + 1
 end;
 ---
-- error: Failed to allocate 268 bytes in slab allocator for tuple
+- error: Failed to allocate 260 bytes in slab allocator for tuple
 ...
 space:len();
 ---
-- 124
+- 120
 ...
 i = 1;
 ---
@@ -43,12 +43,12 @@ while true do
     i = i + 1
 end;
 ---
-- error: Failed to allocate 265 bytes in slab allocator for tuple
+- error: Failed to allocate 257 bytes in slab allocator for tuple
 ...
 --# setopt delimiter ''
 space:len()
 ---
-- 185
+- 179
 ...
 space.index['primary']:get{0}
 ---
-- 
GitLab