Skip to content

Commit 11068c8

Browse files
committed
Minimize GIL hold time: move lock after allocation
Move RedisModule_ThreadSafeContextLock to after MR_ALLOC to prevent blocking Redis operations during memory allocation. The GIL is now only held during the actual memcpy operation, not during allocation. This should help prevent hangs in cluster environments with high concurrency where holding the GIL during allocation could block other Redis operations.
1 parent 12be586 commit 11068c8

File tree

1 file changed

+17
-7
lines changed

1 file changed

+17
-7
lines changed

src/mr.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -584,15 +584,19 @@ static void MR_StepDone(Execution* e, void* pd) {
584584
static void MR_SetRecord(Execution* e, void* pd) {
585585
RedisModuleString* payload = pd;
586586

587-
RedisModule_ThreadSafeContextLock(mr_staticCtx);
588587
size_t dataLen;
589-
const char* data = RedisModule_StringPtrLen(payload, &dataLen);
588+
RedisModule_StringPtrLen(payload, &dataLen);
589+
590590
char* buff_data = MR_ALLOC(dataLen);
591591
if (!buff_data) {
592+
RedisModule_ThreadSafeContextLock(mr_staticCtx);
592593
RedisModule_FreeString(NULL, payload);
593594
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);
594595
return;
595596
}
597+
598+
RedisModule_ThreadSafeContextLock(mr_staticCtx);
599+
const char* data = RedisModule_StringPtrLen(payload, &dataLen);
596600
memcpy(buff_data, data, dataLen);
597601
RedisModule_FreeString(NULL, payload);
598602
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);
@@ -622,14 +626,16 @@ static void MR_SetRecord(Execution* e, void* pd) {
622626

623627
/* Remote function call, runs on the event loop */
624628
static void MR_PassRecord(RedisModuleCtx *ctx, const char *sender_id, uint8_t type, RedisModuleString* payload) {
625-
RedisModule_ThreadSafeContextLock(mr_staticCtx);
626629
size_t dataLen;
627-
const char* data = RedisModule_StringPtrLen(payload, &dataLen);
630+
RedisModule_StringPtrLen(payload, &dataLen);
631+
628632
char* buff_data = MR_ALLOC(ID_LEN);
629633
if (!buff_data) {
630-
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);
631634
return;
632635
}
636+
637+
RedisModule_ThreadSafeContextLock(mr_staticCtx);
638+
const char* data = RedisModule_StringPtrLen(payload, &dataLen);
633639
memcpy(buff_data, data, ID_LEN);
634640
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);
635641

@@ -1129,15 +1135,19 @@ static Execution* MR_ExecutionDeserialize(mr_BufferReader* buffReader) {
11291135
static void MR_RecieveExecution(void* pd) {
11301136
RedisModuleString* payload = pd;
11311137

1132-
RedisModule_ThreadSafeContextLock(mr_staticCtx);
11331138
size_t dataSize;
1134-
const char* data = RedisModule_StringPtrLen(payload, &dataSize);
1139+
RedisModule_StringPtrLen(payload, &dataSize);
1140+
11351141
char* buff_data = MR_ALLOC(dataSize);
11361142
if (!buff_data) {
1143+
RedisModule_ThreadSafeContextLock(mr_staticCtx);
11371144
RedisModule_FreeString(NULL, payload);
11381145
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);
11391146
return;
11401147
}
1148+
1149+
RedisModule_ThreadSafeContextLock(mr_staticCtx);
1150+
const char* data = RedisModule_StringPtrLen(payload, &dataSize);
11411151
memcpy(buff_data, data, dataSize);
11421152
RedisModule_FreeString(NULL, payload);
11431153
RedisModule_ThreadSafeContextUnlock(mr_staticCtx);

0 commit comments

Comments
 (0)