Redis redis-benchmark --threads and -I don't play well together

Not sure whether it is actually practical, but still:

$ redis-benchmark -I --threads 8
Creating 50 idle connections and waiting forever (Ctrl+C when done)
Segmentation fault: 11

/cc @artix75

Comment From: artix75

@itamarhaber I'm gonna check it, thanks.

Comment From: zhouyuan

@artix75 I did some quick tests here, it looks like there's a shortcut in idle mode which will skip the threads creation in benchmark()

diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c
index 23a02d54..5e629700 100644
--- a/src/redis-benchmark.c
+++ b/src/redis-benchmark.c
@@ -1547,6 +1547,14 @@ int main(int argc, const char **argv) {
     if (config.idlemode) {
         printf("Creating %d idle connections and waiting forever (Ctrl+C when done)\n", config.numclients);
         c = createClient("",0,NULL,-1); /* will never receive a reply */
+        if (config.num_threads) {
+            if (config.threads) freeBenchmarkThreads();
+            config.threads = zmalloc(config.num_threads * sizeof(benchmarkThread*));
+            for (i = 0; i < config.num_threads; i++) {
+                benchmarkThread *thread = createBenchmarkThread(i);
+                config.threads[i] = thread;
+            }
+        }
         createMissingClients(c);
         aeMain(config.el);
         /* and will wait for every */

or maybe we should use single thread in idle mode.

diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c
index 23a02d54..2786b2af 100644
--- a/src/redis-benchmark.c
+++ b/src/redis-benchmark.c
@@ -1547,6 +1547,7 @@ int main(int argc, const char **argv) {
     if (config.idlemode) {
         printf("Creating %d idle connections and waiting forever (Ctrl+C when done)\n", config.numclients);
         c = createClient("",0,NULL,-1); /* will never receive a reply */
+        config.num_threads = 0;
         createMissingClients(c);
         aeMain(config.el);
         /* and will wait for every */

Comment From: artix75

@itamarhaber @zhouyuan Fixed it in #5901

Comment From: filipecosta90

@itamarhaber closing this one as @artix75 comment. Furthermore, we now have testing that covers the --threads arg.