[Tarantool-patches] [PATCH 1/1] fiber: extend max fiber name length to 255

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Users keep complaining about too short fiber name. New limit is
255, should be enough for any sane name.

Closes #4394

@TarantoolBot document
Title: fiber.name length limit.

It was 32, now it is 255. Besides, it seems like `fiber.name`
`{truncate = true}` option is not documented.

By default, if a new name is too long, `fiber.name(new_name)`
fails with an exception. To make it always succeed there is an
option truncate: `fiber.name(new_name, {truncate = true})`. It
truncates the name to the max length if it is too long.
---
Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4394-fiber-name
Issue: https://github.com/tarantool/tarantool/issues/4394

@ChangeLog
- Fiber.name length new limit is 255. Previously was 32 (gh-4394).

I made it panic() if set_name fails with OOM, because otherwise it
leads to ugly changes in places, where it was assumed
fiber_set_name() never fails: cord_create() and swim_cfg().

cord_create() can't fail, since is called from a newly created
thread. Allowance to fail would lead to necessity to be able to
return an error from a new thread after it is already started.

swim_cfg() shall not fail at the moment of calling
fiber_set_name(), because otherwise it will make swim_cfg() not
atomic. I then will need something like fiber_reserve_name() to
call before it is too late to fail.

However, I can do these changed if a reviewer wants.

 src/lib/core/fiber.c    | 13 +++++++++++--
 src/lib/core/fiber.h    |  4 ++--
 test/app/fiber.result   |  8 ++++----
 test/app/fiber.test.lua |  2 +-
 test/unit/fiber.cc      |  6 +++---
 test/unit/fiber.result  |  2 +-
 6 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index ada7972cb..abd281dab 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -912,8 +912,13 @@ fiber_loop(MAYBE_UNUSED void *data)
 void
 fiber_set_name(struct fiber *fiber, const char *name)
 {
-	assert(name != NULL);
-	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
+	size_t len = MIN(strlen(name), FIBER_NAME_MAX);
+	char *new_name = realloc(fiber->name, len + 1);
+	if (new_name == NULL)
+		panic("fiber_set_name() can't fail");
+	fiber->name = new_name;
+	memcpy(new_name, name, len);
+	new_name[len] = 0;
 }
 
 static inline void *
@@ -1242,6 +1247,8 @@ fiber_destroy(struct cord *cord, struct fiber *f)
 	region_destroy(&f->gc);
 	fiber_stack_destroy(f, &cord->slabc);
 	diag_destroy(&f->diag);
+	free(f->name);
+	f->name = NULL;
 }
 
 void
@@ -1357,6 +1364,7 @@ cord_create(struct cord *cord, const char *name)
 	fiber_reset(&cord->sched);
 	diag_create(&cord->sched.diag);
 	region_create(&cord->sched.gc, &cord->slabc);
+	cord->sched.name = NULL;
 	fiber_set_name(&cord->sched, "sched");
 	cord->fiber = &cord->sched;
 
@@ -1405,6 +1413,7 @@ cord_destroy(struct cord *cord)
 	}
 	region_destroy(&cord->sched.gc);
 	diag_destroy(&cord->sched.diag);
+	free(cord->sched.name);
 	slab_cache_destroy(&cord->slabc);
 }
 
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 8a3e5aef5..43c5e422e 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -111,7 +111,7 @@ struct cpu_stat {
 
 #endif /* ENABLE_FIBER_TOP */
 
-enum { FIBER_NAME_MAX = 32 };
+enum { FIBER_NAME_MAX = 255 };
 
 /**
  * Fiber ids [0; 100] are reserved.
@@ -510,7 +510,7 @@ struct fiber {
 	struct ipc_wait_pad *wait_pad;
 	/** Exception which caused this fiber's death. */
 	struct diag diag;
-	char name[FIBER_NAME_MAX + 1];
+	char *name;
 };
 
 /** Invoke on_stop triggers and delete them. */
diff --git a/test/app/fiber.result b/test/app/fiber.result
index 6d9604ad8..7331f61b3 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1186,7 +1186,7 @@ fiber = nil
 ---
 ...
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 ---
@@ -1214,14 +1214,14 @@ fiber.name(long_name, {truncate = true})
 ...
 fiber.name()
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 f = fiber.self()
 ---
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 fiber.name(f, 'new_name')
 ---
@@ -1239,7 +1239,7 @@ fiber.name(f, long_name, {truncate = true})
 ...
 fiber.name(f)
 ---
-- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 ...
 --
 -- gh-3493 fiber.create() does not roll back memtx transaction
diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
index 6df210d9c..b8e9abc6e 100644
--- a/test/app/fiber.test.lua
+++ b/test/app/fiber.test.lua
@@ -504,7 +504,7 @@ test_run:cmd("setopt delimiter ''");
 fiber = nil
 
 --
--- gh-2622, gh-4011: fiber.name() truncates new name.
+-- gh-2622, gh-4011, gh-4394: fiber.name() truncates new name.
 --
 fiber = require('fiber')
 long_name = string.rep('a', 300)
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index 91f7d43f9..b0dfc9b14 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -171,9 +171,9 @@ fiber_name_test()
 
 	note("set new fiber name: %s.\n", fiber_name(fiber()));
 
-	const char *long_name = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"\
-		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+	char long_name[FIBER_NAME_MAX + 30];
+	memset(long_name, 'a', sizeof(long_name));
+	long_name[sizeof(long_name) - 1] = 0;
 
 	fiber_set_name(fiber(), long_name);
 
diff --git a/test/unit/fiber.result b/test/unit/fiber.result
index 7c9f85dcd..a61e0a2b8 100644
--- a/test/unit/fiber.result
+++ b/test/unit/fiber.result
@@ -5,7 +5,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
 
 # set new fiber name: Horace.
 
-# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
+# fiber name is truncated: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
 
 	*** fiber_name_test: done ***
 	*** fiber_join_test ***
-- 
2.21.1 (Apple Git-122.3)