Skip to content

Preserve client->id for blocked clients.#6073

Merged
antirez merged 1 commit intoredis:unstablefrom
yossigo:fix/blocked-client-id
May 10, 2019
Merged

Preserve client->id for blocked clients.#6073
antirez merged 1 commit intoredis:unstablefrom
yossigo:fix/blocked-client-id

Conversation

@yossigo
Copy link
Collaborator

@yossigo yossigo commented May 5, 2019

Currently client->id as returned by RM_GetClientId() is wrong for blocked clients.

Hi @antirez I came across unclear behavior in RM_BlockClient() which, if called from Lua or inside MULTI will allocate and return a blocked client but return an error. As it is now it seems like a bug... I would expect the error to be immediate with no blocked client returned (NULL), is there some other (undocumented) intention to this behavior?

@antirez
Copy link
Contributor

antirez commented May 10, 2019

Hello @yossigo, your PR looks good, but about your comment, I think that this was introduced because some module authors forget to declare the command as blocking: without such check you get a crash. Is that what you mean? Thanks.

@antirez antirez merged commit 2903c56 into redis:unstable May 10, 2019
@antirez
Copy link
Contributor

antirez commented May 10, 2019

P.S. I merged this PR regardless of our ongoing discussion @yossigo. Thank you.

@antirez
Copy link
Contributor

antirez commented May 10, 2019

PP.SS. Please would you open an issue about your other finding?

@yossigo yossigo deleted the fix/blocked-client-id branch May 12, 2019 13:41
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Jul 10, 2019
Preserve client->id for blocked clients.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants