when use one redis connection exec "redisCommand"occur err ,the connection will no longer be reused

[Hanliangpan]

see the code below:

     ...
    // first time to use the connection ,command is invalid format
    reply = redisCommand(c,"GET personification_config%_ver");

    if (c->err)
    {
      printf("zhelia 1 Failed to execute cmd err : %s\n", c->errstr);
    }
    else
    {
      printf("GET personification_config_ver: %s\n", reply->str);
    }
    freeReplyObject(reply);

    //    c->err=0;  //  reset the c->err

    // second time to use the connection ,command is correct format
    reply = redisCommand(c,"GET personification_config_ver");
    if (c->err)
    {
      printf("zhelia 2 Failed to execute cmd err : %s\n", c->errstr);
    }
    else
    {
      printf("GET personification_config_ver: %s\n", reply->str);
    }
    freeReplyObject(reply);
    ...

output results:

zhelia 1 Failed to execute cmd err : Invalid format string
zhelia 2 Failed to execute cmd err : Invalid format string

it is amazing！！
why the second time exec redis command get a err.

I use gdb debug this case, find hiread how to deal:

First time, the redis command is "GET personification_config%_ver" , '%' not match any variable parameter list，
and redisvFormatCommand return -2 ,so set c->err and c->errstr;

int redisvAppendCommand(redisContext *c, const char *format, va_list ap) {
    char *cmd;
    int len;

    len = redisvFormatCommand(&cmd,format,ap);
    if (len == -1) {
        __redisSetError(c,REDIS_ERR_OOM,"Out of memory");
        return REDIS_ERR;
    } else if (len == -2) {
        __redisSetError(c,REDIS_ERR_OTHER,"Invalid format string");  //  return here ,  and set c->err and c->errstr
        return REDIS_ERR;
    }

    if (__redisAppendCommand(c,cmd,len) != REDIS_OK) {
        hi_free(cmd);
        return REDIS_ERR;
    }

    hi_free(cmd);
    return REDIS_OK;
}

It's easy to understand that the reason for the first error is that there is no valid string format.

but second time exec command ,why it return the same err info, check the command 'GET personification_config_ver' is ok.

so gdb debug the second redisCommand, I find it return REDIS_OK at "redisvAppendCommand" and not set c->err and c->errstr.
Then debug , I see the second time it return REDIS_ERR at "redisBufferWrite".

the function is used to write the output buffer to the socket. it will check c->err first ,in this case , it return REDIS_ERR.

int redisBufferWrite(redisContext *c, int *done) {

    /* Return early when the context has seen an error. */
    if (c->err)
        return REDIS_ERR;

    if (sdslen(c->obuf) > 0) {
        ssize_t nwritten = c->funcs->write(c);
        if (nwritten < 0) {
            return REDIS_ERR;
        } else if (nwritten > 0) {
            if (nwritten == (ssize_t)sdslen(c->obuf)) {
                sdsfree(c->obuf);
                c->obuf = sdsempty();
                if (c->obuf == NULL)
                    goto oom;
            } else {
                if (sdsrange(c->obuf,nwritten,-1) < 0) goto oom;
            }
        }
    }
    if (done != NULL) *done = (sdslen(c->obuf) == 0);
    return REDIS_OK;

oom:
    __redisSetError(c, REDIS_ERR_OOM, "Out of memory");
    return REDIS_ERR;
}

But where c->err is set to !0, We can easily know the first time we exec 'redisCommand' and set c->err and c->errstr.

the second time use the same connection to exec 'redisCommand' the c->err and c->errstr and not reset , so cause the problem.

And I make a test : reset c->err to 0 between two "redisCommand".

c->err=0;  //  reset the c->err

It work!!! output results:

zhelia 1 Failed to execute cmd err : Invalid format string
GET personification_config_ver: 1662544329118

Maybe you will say, when one connection have err, we should not reused it , best way is rebulid one connect.
but I don't think it's reasonable, there are all err type define, some err type is not fatal, the connection can be reused.

#define REDIS_ERR_IO 1 /* Error in read or write */
#define REDIS_ERR_EOF 3 /* End of file */
#define REDIS_ERR_PROTOCOL 4 /* Protocol error */
#define REDIS_ERR_OOM 5 /* Out of memory */
#define REDIS_ERR_TIMEOUT 6 /* Timed out */
#define REDIS_ERR_OTHER 2 /* Everything else... */

Add another info, I use redis conn pool in multithreaded service, so I will reused the redis connect, when one connect have a err like 'Invalid format string' , it will make more deal request thread err.

When the err appear , and many thread reused the thread to work and cause err .my machine memory goes up rapidly, there is a memory leak, until it crashes because OOM.

English expression is not good, please forgive me. ^_^

[michael-grunder]

but second time exec command ,why it return the same err info, check the command 'GET personification_config_ver' is ok.

I understand your point that a format error probably doesn't need to be a permanently fatal error, but this is just always how hiredis has worked. We don't typically run into this issue as it's rare for people to use dynamic format strings, meaning the error is typically one you can account for at compile-time.

We can look into modifying the behavior for these non-fatal type errors but the trick is always making sure we don't violate backward compatibility.