
Fix #12785 and other race condition issues. See the following isolated comments. The following report was obtained using SANITIZER thread. ```sh make SANITIZER=thread ./runtest-moduleapi --config io-threads 4 --config io-threads-do-reads yes --accurate ``` 1. Fixed thread-safe issue in RM_UnblockClient() Related discussion: https://github.com/redis/redis/pull/12817#issuecomment-1831181220 * When blocking a client in a module using `RM_BlockClientOnKeys()` or `RM_BlockClientOnKeysWithFlags()` with a timeout_callback, calling RM_UnblockClient() in module threads can lead to race conditions in `updateStatsOnUnblock()`. - Introduced: Version: 6.2 PR: #7491 - Touch: `server.stat_numcommands`, `cmd->latency_histogram`, `server.slowlog`, and `server.latency_events` - Harm Level: High Potentially corrupts the memory data of `cmd->latency_histogram`, `server.slowlog`, and `server.latency_events` - Solution: Differentiate whether the call to moduleBlockedClientTimedOut() comes from the module or the main thread. Since we can't know if RM_UnblockClient() comes from module threads, we always assume it does and let `updateStatsOnUnblock()` asynchronously update the unblock status. * When error reply is called in timeout_callback(), ctx is not thread-safe, eventually lead to race conditions in `afterErrorReply`. - Introduced: Version: 6.2 PR: #8217 - Touch `server.stat_total_error_replies`, `server.errors`, - Harm Level: High Potentially corrupts the memory data of `server.errors` - Solution: Make the ctx in `timeout_callback()` with `REDISMODULE_CTX_THREAD_SAFE`, and asynchronously reply errors to the client. 2. Made RM_Reply*() family API thread-safe Related discussion: https://github.com/redis/redis/pull/12817#discussion_r1408707239 Call chain: `RM_Reply*()` -> `_addReplyToBufferOrList()` -> touch server.current_client - Introduced: Version: 7.2.0 PR: #12326 - Harm Level: None Since the module fake client won't have the `CLIENT_PUSHING` flag, even if we touch server.current_client, we can still exit after `c->flags & CLIENT_PUSHING`. - Solution Checking `c->flags & CLIENT_PUSHING` earlier. 3. Made freeClient() thread-safe Fix #12785 - Introduced: Version: 4.0 Commit:3fcf959e60
- Harm Level: Moderate * Trigger assertion It happens when the module thread calls freeClient while the io-thread is in progress, which just triggers an assertion, and doesn't make any race condiaions. * Touch `server.current_client`, `server.stat_clients_type_memory`, and `clientMemUsageBucket->clients`. It happens between the main thread and the module threads, may cause data corruption. 1. Error reset `server.current_client` to NULL, but theoretically this won't happen, because the module has already reset `server.current_client` to old value before entering freeClient. 2. corrupts `clientMemUsageBucket->clients` in updateClientMemUsageAndBucket(). 3. Causes server.stat_clients_type_memory memory statistics to be inaccurate. - Solution: * No longer counts memory usage on fake clients, to avoid updating `server.stat_clients_type_memory` in freeClient. * No longer resetting `server.current_client` in unlinkClient, because the fake client won't be evicted or disconnected in the mid of the process. * Judgment assertion `io_threads_op == IO_THREADS_OP_IDLE` only if c is not a fake client. 4. Fixed free client args without GIL Related discussion: https://github.com/redis/redis/pull/12817#discussion_r1408706695 When freeing retained strings in the module thread (refcount decr), or using them in some way (refcount incr), we should do so while holding the GIL, otherwise, they might be simultaneously freed while the main thread is processing the unblock client state. - Introduced: Version: 6.2.0 PR: #8141 - Harm Level: Low Trigger assertion or double free or memory leak. - Solution: Documenting that module API users need to ensure any access to these retained strings is done with the GIL locked 5. Fix adding fake client to server.clients_pending_write It will incorrectly log the memory usage for the fake client. Related discussion: https://github.com/redis/redis/pull/12817#issuecomment-1851899163 - Introduced: Version: 4.0 Commit:9b01b64430
- Harm Level: None Only result in NOP - Solution: * Don't add fake client into server.clients_pending_write * Add c->conn assertion for updateClientMemUsageAndBucket() and updateClientMemoryUsage() to avoid same issue in the future. So now it will be the responsibility of the caller of both of them to avoid passing in fake client. 6. Fix calling RM_BlockedClientMeasureTimeStart() and RM_BlockedClientMeasureTimeEnd() without GIL - Introduced: Version: 6.2 PR: #7491 - Harm Level: Low Causes inaccuracies in command latency histogram and slow logs, but does not corrupt memory. - Solution: Module API users, if know that non-thread-safe APIs will be used in multi-threading, need to take responsibility for protecting them with their own locks instead of the GIL, as using the GIL is too expensive. ### Other issue 1. RM_Yield is not thread-safe, fixed via #12905. ### Summarize 1. Fix thread-safe issues for `RM_UnblockClient()`, `freeClient()` and `RM_Yield`, potentially preventing memory corruption, data disorder, or assertion. 2. Updated docs and module test to clarify module API users' responsibility for locking non-thread-safe APIs in multi-threading, such as RM_BlockedClientMeasureTimeStart/End(), RM_FreeString(), RM_RetainString(), and RM_HoldString(). ### About backpot to 7.2 1. The implement of (1) is not too satisfying, would like to get more eyes. 2. (2), (3) can be safely for backport 3. (4), (6) just modifying the module tests and updating the documentation, no need for a backpot. 4. (5) is harmless, no need for a backpot. --------- Co-authored-by: Oran Agra <oran@redislabs.com>
331 lines
13 KiB
C
331 lines
13 KiB
C
#define _XOPEN_SOURCE 700
|
|
#include "redismodule.h"
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <pthread.h>
|
|
#include <time.h>
|
|
|
|
#define UNUSED(x) (void)(x)
|
|
|
|
typedef struct {
|
|
/* Mutex for protecting RedisModule_BlockedClientMeasureTime*() API from race
|
|
* conditions due to timeout callback triggered in the main thread. */
|
|
pthread_mutex_t measuretime_mutex;
|
|
int measuretime_completed; /* Indicates that time measure has ended and will not continue further */
|
|
int myint; /* Used for replying */
|
|
} BlockPrivdata;
|
|
|
|
void blockClientPrivdataInit(RedisModuleBlockedClient *bc) {
|
|
BlockPrivdata *block_privdata = RedisModule_Calloc(1, sizeof(*block_privdata));
|
|
block_privdata->measuretime_mutex = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;
|
|
RedisModule_BlockClientSetPrivateData(bc, block_privdata);
|
|
}
|
|
|
|
void blockClientMeasureTimeStart(RedisModuleBlockedClient *bc, BlockPrivdata *block_privdata) {
|
|
pthread_mutex_lock(&block_privdata->measuretime_mutex);
|
|
RedisModule_BlockedClientMeasureTimeStart(bc);
|
|
pthread_mutex_unlock(&block_privdata->measuretime_mutex);
|
|
}
|
|
|
|
void blockClientMeasureTimeEnd(RedisModuleBlockedClient *bc, BlockPrivdata *block_privdata, int completed) {
|
|
pthread_mutex_lock(&block_privdata->measuretime_mutex);
|
|
if (!block_privdata->measuretime_completed) {
|
|
RedisModule_BlockedClientMeasureTimeEnd(bc);
|
|
if (completed) block_privdata->measuretime_completed = 1;
|
|
}
|
|
pthread_mutex_unlock(&block_privdata->measuretime_mutex);
|
|
}
|
|
|
|
/* Reply callback for blocking command BLOCK.DEBUG */
|
|
int HelloBlock_Reply(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
UNUSED(argv);
|
|
UNUSED(argc);
|
|
BlockPrivdata *block_privdata = RedisModule_GetBlockedClientPrivateData(ctx);
|
|
return RedisModule_ReplyWithLongLong(ctx,block_privdata->myint);
|
|
}
|
|
|
|
/* Timeout callback for blocking command BLOCK.DEBUG */
|
|
int HelloBlock_Timeout(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
UNUSED(argv);
|
|
UNUSED(argc);
|
|
RedisModuleBlockedClient *bc = RedisModule_GetBlockedClientHandle(ctx);
|
|
BlockPrivdata *block_privdata = RedisModule_GetBlockedClientPrivateData(ctx);
|
|
blockClientMeasureTimeEnd(bc, block_privdata, 1);
|
|
return RedisModule_ReplyWithSimpleString(ctx,"Request timedout");
|
|
}
|
|
|
|
/* Private data freeing callback for BLOCK.DEBUG command. */
|
|
void HelloBlock_FreeData(RedisModuleCtx *ctx, void *privdata) {
|
|
UNUSED(ctx);
|
|
BlockPrivdata *block_privdata = privdata;
|
|
pthread_mutex_destroy(&block_privdata->measuretime_mutex);
|
|
RedisModule_Free(privdata);
|
|
}
|
|
|
|
/* Private data freeing callback for BLOCK.BLOCK command. */
|
|
void HelloBlock_FreeStringData(RedisModuleCtx *ctx, void *privdata) {
|
|
RedisModule_FreeString(ctx, (RedisModuleString*)privdata);
|
|
}
|
|
|
|
/* The thread entry point that actually executes the blocking part
|
|
* of the command BLOCK.DEBUG. */
|
|
void *BlockDebug_ThreadMain(void *arg) {
|
|
void **targ = arg;
|
|
RedisModuleBlockedClient *bc = targ[0];
|
|
long long delay = (unsigned long)targ[1];
|
|
long long enable_time_track = (unsigned long)targ[2];
|
|
BlockPrivdata *block_privdata = RedisModule_BlockClientGetPrivateData(bc);
|
|
|
|
if (enable_time_track)
|
|
blockClientMeasureTimeStart(bc, block_privdata);
|
|
RedisModule_Free(targ);
|
|
|
|
struct timespec ts;
|
|
ts.tv_sec = delay / 1000;
|
|
ts.tv_nsec = (delay % 1000) * 1000000;
|
|
nanosleep(&ts, NULL);
|
|
if (enable_time_track)
|
|
blockClientMeasureTimeEnd(bc, block_privdata, 0);
|
|
block_privdata->myint = rand();
|
|
RedisModule_UnblockClient(bc,block_privdata);
|
|
return NULL;
|
|
}
|
|
|
|
/* The thread entry point that actually executes the blocking part
|
|
* of the command BLOCK.DOUBLE_DEBUG. */
|
|
void *DoubleBlock_ThreadMain(void *arg) {
|
|
void **targ = arg;
|
|
RedisModuleBlockedClient *bc = targ[0];
|
|
long long delay = (unsigned long)targ[1];
|
|
BlockPrivdata *block_privdata = RedisModule_BlockClientGetPrivateData(bc);
|
|
blockClientMeasureTimeStart(bc, block_privdata);
|
|
RedisModule_Free(targ);
|
|
struct timespec ts;
|
|
ts.tv_sec = delay / 1000;
|
|
ts.tv_nsec = (delay % 1000) * 1000000;
|
|
nanosleep(&ts, NULL);
|
|
blockClientMeasureTimeEnd(bc, block_privdata, 0);
|
|
/* call again RedisModule_BlockedClientMeasureTimeStart() and
|
|
* RedisModule_BlockedClientMeasureTimeEnd and ensure that the
|
|
* total execution time is 2x the delay. */
|
|
blockClientMeasureTimeStart(bc, block_privdata);
|
|
nanosleep(&ts, NULL);
|
|
blockClientMeasureTimeEnd(bc, block_privdata, 0);
|
|
block_privdata->myint = rand();
|
|
RedisModule_UnblockClient(bc,block_privdata);
|
|
return NULL;
|
|
}
|
|
|
|
void HelloBlock_Disconnected(RedisModuleCtx *ctx, RedisModuleBlockedClient *bc) {
|
|
RedisModule_Log(ctx,"warning","Blocked client %p disconnected!",
|
|
(void*)bc);
|
|
}
|
|
|
|
/* BLOCK.DEBUG <delay_ms> <timeout_ms> -- Block for <count> milliseconds, then reply with
|
|
* a random number. Timeout is the command timeout, so that you can test
|
|
* what happens when the delay is greater than the timeout. */
|
|
int HelloBlock_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
if (argc != 3) return RedisModule_WrongArity(ctx);
|
|
long long delay;
|
|
long long timeout;
|
|
|
|
if (RedisModule_StringToLongLong(argv[1],&delay) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx,"ERR invalid count");
|
|
}
|
|
|
|
if (RedisModule_StringToLongLong(argv[2],&timeout) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx,"ERR invalid count");
|
|
}
|
|
|
|
pthread_t tid;
|
|
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,HelloBlock_Reply,HelloBlock_Timeout,HelloBlock_FreeData,timeout);
|
|
blockClientPrivdataInit(bc);
|
|
|
|
/* Here we set a disconnection handler, however since this module will
|
|
* block in sleep() in a thread, there is not much we can do in the
|
|
* callback, so this is just to show you the API. */
|
|
RedisModule_SetDisconnectCallback(bc,HelloBlock_Disconnected);
|
|
|
|
/* Now that we setup a blocking client, we need to pass the control
|
|
* to the thread. However we need to pass arguments to the thread:
|
|
* the delay and a reference to the blocked client handle. */
|
|
void **targ = RedisModule_Alloc(sizeof(void*)*3);
|
|
targ[0] = bc;
|
|
targ[1] = (void*)(unsigned long) delay;
|
|
// pass 1 as flag to enable time tracking
|
|
targ[2] = (void*)(unsigned long) 1;
|
|
|
|
if (pthread_create(&tid,NULL,BlockDebug_ThreadMain,targ) != 0) {
|
|
RedisModule_AbortBlock(bc);
|
|
return RedisModule_ReplyWithError(ctx,"-ERR Can't start thread");
|
|
}
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
/* BLOCK.DEBUG_NOTRACKING <delay_ms> <timeout_ms> -- Block for <count> milliseconds, then reply with
|
|
* a random number. Timeout is the command timeout, so that you can test
|
|
* what happens when the delay is greater than the timeout.
|
|
* this command does not track background time so the background time should no appear in stats*/
|
|
int HelloBlockNoTracking_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
if (argc != 3) return RedisModule_WrongArity(ctx);
|
|
long long delay;
|
|
long long timeout;
|
|
|
|
if (RedisModule_StringToLongLong(argv[1],&delay) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx,"ERR invalid count");
|
|
}
|
|
|
|
if (RedisModule_StringToLongLong(argv[2],&timeout) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx,"ERR invalid count");
|
|
}
|
|
|
|
pthread_t tid;
|
|
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,HelloBlock_Reply,HelloBlock_Timeout,HelloBlock_FreeData,timeout);
|
|
blockClientPrivdataInit(bc);
|
|
|
|
/* Here we set a disconnection handler, however since this module will
|
|
* block in sleep() in a thread, there is not much we can do in the
|
|
* callback, so this is just to show you the API. */
|
|
RedisModule_SetDisconnectCallback(bc,HelloBlock_Disconnected);
|
|
|
|
/* Now that we setup a blocking client, we need to pass the control
|
|
* to the thread. However we need to pass arguments to the thread:
|
|
* the delay and a reference to the blocked client handle. */
|
|
void **targ = RedisModule_Alloc(sizeof(void*)*3);
|
|
targ[0] = bc;
|
|
targ[1] = (void*)(unsigned long) delay;
|
|
// pass 0 as flag to enable time tracking
|
|
targ[2] = (void*)(unsigned long) 0;
|
|
|
|
if (pthread_create(&tid,NULL,BlockDebug_ThreadMain,targ) != 0) {
|
|
RedisModule_AbortBlock(bc);
|
|
return RedisModule_ReplyWithError(ctx,"-ERR Can't start thread");
|
|
}
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
/* BLOCK.DOUBLE_DEBUG <delay_ms> -- Block for 2 x <count> milliseconds,
|
|
* then reply with a random number.
|
|
* This command is used to test multiple calls to RedisModule_BlockedClientMeasureTimeStart()
|
|
* and RedisModule_BlockedClientMeasureTimeEnd() within the same execution. */
|
|
int HelloDoubleBlock_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
if (argc != 2) return RedisModule_WrongArity(ctx);
|
|
long long delay;
|
|
|
|
if (RedisModule_StringToLongLong(argv[1],&delay) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx,"ERR invalid count");
|
|
}
|
|
|
|
pthread_t tid;
|
|
RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx,HelloBlock_Reply,HelloBlock_Timeout,HelloBlock_FreeData,0);
|
|
blockClientPrivdataInit(bc);
|
|
|
|
/* Now that we setup a blocking client, we need to pass the control
|
|
* to the thread. However we need to pass arguments to the thread:
|
|
* the delay and a reference to the blocked client handle. */
|
|
void **targ = RedisModule_Alloc(sizeof(void*)*2);
|
|
targ[0] = bc;
|
|
targ[1] = (void*)(unsigned long) delay;
|
|
|
|
if (pthread_create(&tid,NULL,DoubleBlock_ThreadMain,targ) != 0) {
|
|
RedisModule_AbortBlock(bc);
|
|
return RedisModule_ReplyWithError(ctx,"-ERR Can't start thread");
|
|
}
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
RedisModuleBlockedClient *blocked_client = NULL;
|
|
|
|
/* BLOCK.BLOCK [TIMEOUT] -- Blocks the current client until released
|
|
* or TIMEOUT seconds. If TIMEOUT is zero, no timeout function is
|
|
* registered.
|
|
*/
|
|
int Block_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
if (RedisModule_IsBlockedReplyRequest(ctx)) {
|
|
RedisModuleString *r = RedisModule_GetBlockedClientPrivateData(ctx);
|
|
return RedisModule_ReplyWithString(ctx, r);
|
|
} else if (RedisModule_IsBlockedTimeoutRequest(ctx)) {
|
|
RedisModule_UnblockClient(blocked_client, NULL); /* Must be called to avoid leaks. */
|
|
blocked_client = NULL;
|
|
return RedisModule_ReplyWithSimpleString(ctx, "Timed out");
|
|
}
|
|
|
|
if (argc != 2) return RedisModule_WrongArity(ctx);
|
|
long long timeout;
|
|
|
|
if (RedisModule_StringToLongLong(argv[1], &timeout) != REDISMODULE_OK) {
|
|
return RedisModule_ReplyWithError(ctx, "ERR invalid timeout");
|
|
}
|
|
if (blocked_client) {
|
|
return RedisModule_ReplyWithError(ctx, "ERR another client already blocked");
|
|
}
|
|
|
|
/* Block client. We use this function as both a reply and optional timeout
|
|
* callback and differentiate the different code flows above.
|
|
*/
|
|
blocked_client = RedisModule_BlockClient(ctx, Block_RedisCommand,
|
|
timeout > 0 ? Block_RedisCommand : NULL, HelloBlock_FreeStringData, timeout);
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
/* BLOCK.IS_BLOCKED -- Returns 1 if we have a blocked client, or 0 otherwise.
|
|
*/
|
|
int IsBlocked_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
UNUSED(argv);
|
|
UNUSED(argc);
|
|
RedisModule_ReplyWithLongLong(ctx, blocked_client ? 1 : 0);
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
/* BLOCK.RELEASE [reply] -- Releases the blocked client and produce the specified reply.
|
|
*/
|
|
int Release_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
if (argc != 2) return RedisModule_WrongArity(ctx);
|
|
if (!blocked_client) {
|
|
return RedisModule_ReplyWithError(ctx, "ERR No blocked client");
|
|
}
|
|
|
|
RedisModuleString *replystr = argv[1];
|
|
RedisModule_RetainString(ctx, replystr);
|
|
RedisModule_UnblockClient(blocked_client, replystr);
|
|
blocked_client = NULL;
|
|
|
|
RedisModule_ReplyWithSimpleString(ctx, "OK");
|
|
|
|
return REDISMODULE_OK;
|
|
}
|
|
|
|
int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
|
|
UNUSED(argv);
|
|
UNUSED(argc);
|
|
|
|
if (RedisModule_Init(ctx,"block",1,REDISMODULE_APIVER_1)
|
|
== REDISMODULE_ERR) return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx,"block.debug",
|
|
HelloBlock_RedisCommand,"",0,0,0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx,"block.double_debug",
|
|
HelloDoubleBlock_RedisCommand,"",0,0,0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx,"block.debug_no_track",
|
|
HelloBlockNoTracking_RedisCommand,"",0,0,0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx, "block.block",
|
|
Block_RedisCommand, "", 0, 0, 0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx,"block.is_blocked",
|
|
IsBlocked_RedisCommand,"",0,0,0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
if (RedisModule_CreateCommand(ctx,"block.release",
|
|
Release_RedisCommand,"",0,0,0) == REDISMODULE_ERR)
|
|
return REDISMODULE_ERR;
|
|
|
|
return REDISMODULE_OK;
|
|
}
|