To reproduce:

  1. Block the client.
  2. Create a thread safe context from the blocked client.
  3. Call RedisModule_WrongArity(ctx)

The reason is that the function does the following:


 addReplyErrorFormat(ctx->client,
        "wrong number of arguments for '%s' command",
        (char*)ctx->client->argv[0]->ptr);

and we do not have argv at this point. I think the simplest thing to do would be to keep a pointer to the command name (with incrRef) directly in the context and not rely on argv and the client. It might come in handy in other places as well.

Comment From: dvirsky

cc: @mnunberg if you have anything to add to this...

Comment From: mnunberg

This is because it tries to dereference the argv[0] in the internal client in order to get the original command name. Because the internal client in the thread safe context lacks the array, it crashes.

It seems we can either

Omit the command name from the error message itself Copy just the command name (wasteful IMO) Make the argv array itself thread safe (read-only though) - this would also allow us to avoid internal copying especially during indexing.

On Mar 15, 2018, at 6:08 AM, Dvir Volk notifications@github.com wrote:

cc: @mnunberg https://github.com/mnunberg if you have anything to add to this...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/antirez/redis/issues/4756#issuecomment-373324452, or mute the thread https://github.com/notifications/unsubscribe-auth/AAanUb8fKN2FKKxde0gyCmYg5YAR3DNwks5tej2xgaJpZM4Sr3Pe.

Comment From: dvirsky

Keep in mind that we don't have to actually copy the string. We can copy the robj and increase the ref count, which is very cheap.

Comment From: jonahharris

Agreed. @dvirsky can you share your exact example case. I want to run some tests on different methods regarding the fix.

Comment From: dvirsky

@jonahharris I don't have anything ATM, we had it in redisearch but fixed it. You can modify the module examples in the redis source tree to reproduce this pretty easily. try helloblock.c I think.