Unverified Commit 6016973a authored by Binbin's avatar Binbin Committed by GitHub
Browse files

Fix module assertion crash when timer and timeout are unlocked in the same event loop (#13015)

When we use a timer to unblock a client in module, if the timer
period and the block timeout are very close, they will unblock the
client in the same event loop, and it will trigger the assertion.
The reason is that in moduleBlockedClientTimedOut we will protect
against re-processing, so we don't actually call updateStatsOnUnblock
(see #12817), so we are not able to reset the c->duration. 

The reason is unblockClientOnTimeout() didn't realize that bc had
been unblocked. We add a function to the module to determine if bc
is blocked, and then use it in unblockClientOnTimeout() to exit.

There is the stack:
```
beforeSleep
blockedBeforeSleep
handleBlockedClientsTimeout
checkBlockedClientTimeout
unblockClientOnTimeout
unblockClient
resetClient
-- assertion, crash the server
'c->duration == 0' is not true
```
parent 74a6e48a
...@@ -707,6 +707,9 @@ static void moduleUnblockClientOnKey(client *c, robj *key) { ...@@ -707,6 +707,9 @@ static void moduleUnblockClientOnKey(client *c, robj *key) {
* we want to remove the pending flag to indicate we already responded to the * we want to remove the pending flag to indicate we already responded to the
* command with timeout reply. */ * command with timeout reply. */
void unblockClientOnTimeout(client *c) { void unblockClientOnTimeout(client *c) {
/* The client has been unlocked (in the moduleUnblocked list), return ASAP. */
if (c->bstate.btype == BLOCKED_MODULE && isModuleClientUnblocked(c)) return;
replyToBlockedClientTimedOut(c); replyToBlockedClientTimedOut(c);
if (c->flags & CLIENT_PENDING_COMMAND) if (c->flags & CLIENT_PENDING_COMMAND)
c->flags &= ~CLIENT_PENDING_COMMAND; c->flags &= ~CLIENT_PENDING_COMMAND;
......
...@@ -7698,6 +7698,13 @@ void RM_LatencyAddSample(const char *event, mstime_t latency) { ...@@ -7698,6 +7698,13 @@ void RM_LatencyAddSample(const char *event, mstime_t latency) {
* https://redis.io/topics/modules-blocking-ops. * https://redis.io/topics/modules-blocking-ops.
* -------------------------------------------------------------------------- */ * -------------------------------------------------------------------------- */
   
/* Returns 1 if the client already in the moduleUnblocked list, 0 otherwise. */
int isModuleClientUnblocked(client *c) {
RedisModuleBlockedClient *bc = c->bstate.module_blocked_handle;
return bc->unblocked == 1;
}
/* This is called from blocked.c in order to unblock a client: may be called /* This is called from blocked.c in order to unblock a client: may be called
* for multiple reasons while the client is in the middle of being blocked * for multiple reasons while the client is in the middle of being blocked
* because the client is terminated, but is also called for cleanup when a * because the client is terminated, but is also called for cleanup when a
......
...@@ -2532,6 +2532,7 @@ const char *moduleTypeModuleName(moduleType *mt); ...@@ -2532,6 +2532,7 @@ const char *moduleTypeModuleName(moduleType *mt);
const char *moduleNameFromCommand(struct redisCommand *cmd); const char *moduleNameFromCommand(struct redisCommand *cmd);
void moduleFreeContext(struct RedisModuleCtx *ctx); void moduleFreeContext(struct RedisModuleCtx *ctx);
void moduleCallCommandUnblockedHandler(client *c); void moduleCallCommandUnblockedHandler(client *c);
int isModuleClientUnblocked(client *c);
void unblockClientFromModule(client *c); void unblockClientFromModule(client *c);
void moduleHandleBlockedClients(void); void moduleHandleBlockedClients(void);
void moduleBlockedClientTimedOut(client *c, int from_module); void moduleBlockedClientTimedOut(client *c, int from_module);
......
...@@ -290,6 +290,12 @@ foreach call_type {nested normal} { ...@@ -290,6 +290,12 @@ foreach call_type {nested normal} {
after 120 after 120
$rd close $rd close
} }
test {block time is equal to timer period} {
# These time is equal, they will be unlocked in the same event loop,
# when the client is unlock, we will get the OK reply from timer.
assert_match "OK" [r unblock_by_timer 100 100]
}
test "Unload the module - blockedclient" { test "Unload the module - blockedclient" {
assert_equal {OK} [r module unload blockedclient] assert_equal {OK} [r module unload blockedclient]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment