Would it be problem invoking ValkeyModule_UnlinkKey in separated thread while main thread is opening same key?

[zh1029]

We meet problem looks that valkey server main thread is in busy loop which can't respond ping-pong message. Below is GDB trace for example:

Program terminated with signal SIGABRT, Aborted.
#0 0x0000007fbf17aa0c in __GI_kill () at ../sysdeps/unix/syscall-template.S:120
120 ../sysdeps/unix/syscall-template.S: No such file or directory.
[Current thread is 1 (LWP 16)]
(gdb) bt
#0 0x0000007fbf17aa0c in __GI_kill () at ../sysdeps/unix/syscall-template.S:120
#1 0x00000000004c5c0c in bugReportEnd ()
#2
#3 0x000000000044647c in dictFind ()
#4 0x000000000047928c in lookupKey ()
#5 0x000000000051181c in VM_OpenKey ()
#6 0x0000007fbf09197c in setPubStringWithTagsCommon (ctx=ctx@entry=warning: could not convert 'ValkeyModuleCtx' from the host encoding (ANSI_X3.4-1968) to UTF-32.
This normally should not happen, please file a bug report.
0x7fc2324bd0, setParamsPtr=setParamsPtr@entry=0x7fc2324ba0,
pubParamsPtr=pubParamsPtr@entry=0x7fc2324bb0, tagParamsPtr=tagParamsPtr@entry=0x7fc2324bc0, reliable=230, reliable@entry=false,
single=single@entry=false) at ../redismodule-1.53.11/src/exstring/exstrings.c:2101
#7 0x0000007fbf093390 in setMPubStringCommon (ctx=0x7fc2324bd0, argv=0x297c28e0, argc=, reliable=)
at ../redismodule-1.53.11/src/exstring/exstrings.c:2316
#8 0x000000000050c584 in ValkeyModuleCommandDispatcher ()
#9 0x0000000000453220 in call ()
#10 0x0000000000453bfc in processCommand ()
#11 0x000000000046d67c in processInputBuffer ()
#12 0x0000000000500510 in processUnblockedClients ()
#13 0x000000000044e964 in beforeSleep ()
#14 0x000000000044a340 in ?? ()
#15 0x000000000044a82c in aeMain ()
#16 0x0000000000440c14 in main ()
(gdb) info thre
Id Target Id Frame

• 1 LWP 16 0x0000007fbf17aa0c in __GI_kill () at ../sysdeps/unix/syscall-template.S:120
  2 LWP 22 futex_wait (private=0, expected=2, futex_word=0x675908)
  at /usr/src/debug/glibc/2.38+git-r0/sysdeps/nptl/futex-internal.h:146
  (gdb) f 2
  Updated security.md #2
  (gdb) t 2
  [Switching to thread 2 (LWP 22)]
  #0 futex_wait (private=0, expected=2, futex_word=0x675908) at /usr/src/debug/glibc/2.38+git-r0/sysdeps/nptl/futex-internal.h:146
  146 /usr/src/debug/glibc/2.38+git-r0/sysdeps/nptl/futex-internal.h: No such file or directory.
  (gdb) bt
  #0 futex_wait (private=0, expected=2, futex_word=0x675908) at /usr/src/debug/glibc/2.38+git-r0/sysdeps/nptl/futex-internal.h:146
  Test #1 __GI___lll_lock_wait (futex=futex@entry=0x675908, private=0) at lowlevellock.c:49
  Updated security.md #2 0x0000007fbf1c0944 in lll_mutex_lock_optimized (mutex=0x675908) at pthread_mutex_lock.c:48
  Support for key locking  #3 ___pthread_mutex_lock (mutex=0x675908) at pthread_mutex_lock.c:93
  SCAN, SSCAN, HSCAN, and ZSCAN are not stable across failovers #4 0x000000000050e688 in VM_ThreadSafeContextLock ()
  Update CODE_OF_CONDUCT.md #5 0x0000007fbf094a9c in scanKeyAndDelete (ctx=ctx@entry=0x7fb8000b70, pattern=, maxString=,
  locked=locked@entry=true, bc=0x7fbcffe7c8, bc@entry=0x297b6300, delNumber=0x7fbcffe7bc, delNumber@entry=0x7fbcffe8a8)
  at ../redismodule-1.53.11/src/exstring/exstrings.c:1158
  Fix daily.yml to not run tests against unstable #6 0x0000007fbf094ed4 in NdelGenericCommand (ctx=ctx@entry=0x7fb8000b70, argv=argv@entry=0x2982a610, argc=argc@entry=11,
  locked=locked@entry=true, bc=bc@entry=0x297b6300) at ../redismodule-1.53.11/src/exstring/exstrings.c:1271
  Update SECURITY.md #7 0x0000007fbf0950e0 in Generic_NoAtomic_ThreadMain (arg=0x298e3df0) at ../redismodule-1.53.11/src/exstring/exstrings.c:1872
  [QUESTION] Fork status #8 0x0000007fbf1bdaa4 in start_thread (arg=0x0) at pthread_create.c:447
  Renaming thread #9 0x0000007fbf22434c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone3.S:76
  (gdb)

The child thread is deleting multiple keys matching pattern via ValkeyModule_UnlinkKey. while main thread is hanging in opening key.

I pasted part of code.

int scanKeyAndDelete(RedisModuleCtx* ctx, RedisModuleString* pattern, RedisModuleString* maxString,
bool locked, RedisModuleBlockedClient* bc, long long* delNumber)
{
long long replylen = 0;
long long count = 0;
long long max = 0;
int replyType = REPLY_WITH_LONGLONG;
int ret = REDISMODULE_OK;
RedisModuleKey* key;

......
int retArgc = 0;
RedisModuleString* retArgv[max];
memset(retArgv, 0, sizeof(retArgv));

ScanKeyPrivateData pd = {
    .ctx = ctx,
    .pattern = pattern,
    .length = &replylen,
    .flag = SCAN_TO_DEL,
    .retArgv = &retArgv[0],
    .retArgc = &retArgc,
    .maxRetArgc = &max,
};

bool historyStarted = isHistoryStarted(ctx);
bool keyHistoryStarted = false;
RedisModuleString* oldValue = NULL;

RedisModuleScanCursor* cursor = RedisModule_ScanCursorCreate();

RM_THREADSAFECONTEXTLOCK()
while (RedisModule_Scan(ctx, cursor, scanKeyCallBack, &pd))
{
    if (retArgc)
    {
        /*There is case return multiple keys from different namespace in one RedisModule_Scan call.
          So remove key one by one*/
        for (int i = 0; i < retArgc; i++)
        {
            if (historyStarted)
                keyHistoryStarted = isKeyHistoryStarted(ctx, retArgv[i]);

            DelParams delParams = {
                .keys = &retArgv[i],
                .length = 1};
            delPubStringFromTags(ctx, &delParams);

            key = RedisModule_OpenKey(ctx, retArgv[i], REDISMODULE_WRITE);
            if (keyHistoryStarted)
            {
                size_t oldValLen = 0;
                char* val = RedisModule_StringDMA(key, &oldValLen, REDISMODULE_READ);
                oldValue = RedisModule_CreateString(ctx, val, oldValLen);
            }
            replylen += (**RedisModule_UnlinkKey(key)** == REDISMODULE_OK) ? 1 : 0;
            RM_CLOSEKEY(key);

            RedisModule_FreeString(ctx, retArgv[i]);
        }
        retArgc = 0;
    }

    if (++count >= max)
    {
        count = 0;
        RM_THREADSAFECONTEXTUNLOCK()
        RM_THREADSAFECONTEXTLOCK()
    }
}

......
RM_THREADSAFECONTEXTUNLOCK()
return ret;
}

int setPubStringWithTagsCommon(RedisModuleCtx* ctx, SetParams* setParamsPtr, PubParams* pubParamsPtr, TagParams* tagParamsPtr,
bool reliable, bool single)
{
int ret = REDISMODULE_OK;
......
RedisModuleString* oldVal = NULL;

for (size_t i = 0; i < setParamsPtr->length; i += 2)
{

    RedisModuleKey* key = **RedisModule_OpenKey**(ctx, setParamsPtr->key_val_pairs[i], REDISMODULE_READ | REDISMODULE_WRITE);

    ret = RedisModule_StringSet(key, setParamsPtr->key_val_pairs[i + 1]);
    if (ret != REDISMODULE_OK)
    {
        RedisModule_ReplyWithError(ctx, "-ERR Failed to set key");
        RM_CLOSEKEY(key);
        return ret;
    }
    RM_CLOSEKEY(key);

......
}

if (!pubParamsPtr)
{
    RedisModule_ReplyWithSimpleString(ctx, "OK");
    return REDISMODULE_OK;
}

ChannelMessagePublishToClientsNumber pNotifyClientNum[pubParamsPtr->length / 2];
memset(pNotifyClientNum, 0, sizeof(pNotifyClientNum));
int clientNum = 0;
if (multiPublishAndGetNotifyClientNumber(ctx, pubParamsPtr->channel_msg_pairs, pubParamsPtr->length, reliable, single, &pNotifyClientNum[0], &clientNum) >= 0)
    setReplyOfSetWithTagsPubCommand(ctx, setParamsPtr, clientNum, &pNotifyClientNum[0]);

return REDISMODULE_OK;

}

[zh1029]

Close the discussing after raising one issue #1874