Fix possible crash in prefetchKeysAsync with flash enabled. (#578)

* remove short circuit as it is unsafe
This commit is contained in:
Malavan Sotheeswaran 2023-02-22 19:27:24 -05:00 committed by GitHub
parent 4ac449aee7
commit 81c6c7902d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 9 additions and 21 deletions

View File

@ -3265,7 +3265,7 @@ int dbnumFromDb(redisDb *db)
serverPanic("invalid database pointer"); serverPanic("invalid database pointer");
} }
bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command, bool fExecOK) void redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command)
{ {
if (m_spstorage == nullptr) { if (m_spstorage == nullptr) {
#if defined(__x86_64__) || defined(__i386__) #if defined(__x86_64__) || defined(__i386__)
@ -3277,7 +3277,7 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
const char *cmd = szFromObj(command.argv[0]); const char *cmd = szFromObj(command.argv[0]);
if (!strcasecmp(cmd, "set") || !strcasecmp(cmd, "get")) { if (!strcasecmp(cmd, "set") || !strcasecmp(cmd, "get")) {
if (c->db->m_spdbSnapshotHOLDER != nullptr) if (c->db->m_spdbSnapshotHOLDER != nullptr)
return false; // this is dangerous enough without a snapshot around return; // this is dangerous enough without a snapshot around
auto h = dictSdsHash(szFromObj(command.argv[1])); auto h = dictSdsHash(szFromObj(command.argv[1]));
for (int iht = 0; iht < 2; ++iht) { for (int iht = 0; iht < 2; ++iht) {
auto hT = h & c->db->m_pdict->ht[iht].sizemask; auto hT = h & c->db->m_pdict->ht[iht].sizemask;
@ -3297,7 +3297,7 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
} }
} }
#endif #endif
return false; return;
} }
AeLocker lock; AeLocker lock;
@ -3307,10 +3307,10 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
getKeysResult result = GETKEYS_RESULT_INIT; getKeysResult result = GETKEYS_RESULT_INIT;
auto cmd = lookupCommand(szFromObj(command.argv[0])); auto cmd = lookupCommand(szFromObj(command.argv[0]));
if (cmd == nullptr) if (cmd == nullptr)
return false; // Bad command? It's not for us to judge, just bail return; // Bad command? It's not for us to judge, just bail
if (command.argc < std::abs(cmd->arity)) if (command.argc < std::abs(cmd->arity))
return false; // Invalid number of args return; // Invalid number of args
int numkeys = getKeysFromCommand(cmd, command.argv, command.argc, &result); int numkeys = getKeysFromCommand(cmd, command.argv, command.argc, &result);
for (int ikey = 0; ikey < numkeys; ++ikey) for (int ikey = 0; ikey < numkeys; ++ikey)
@ -3343,7 +3343,6 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
} }
} }
bool fNoInsert = false;
if (!vecInserts.empty()) { if (!vecInserts.empty()) {
lock.arm(c); lock.arm(c);
for (auto &tuple : vecInserts) for (auto &tuple : vecInserts)
@ -3359,13 +3358,11 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
// While unlocked this was already ensured // While unlocked this was already ensured
decrRefCount(o); decrRefCount(o);
sdsfree(sharedKey); sdsfree(sharedKey);
fNoInsert = true;
} }
else else
{ {
if (spexpire != nullptr) { if (spexpire != nullptr) {
if (spexpire->when() < mstime()) { if (spexpire->when() < mstime()) {
fNoInsert = true;
break; break;
} }
} }
@ -3392,12 +3389,5 @@ bool redisDbPersistentData::prefetchKeysAsync(client *c, parsed_command &command
lock.disarm(); lock.disarm();
} }
if (fExecOK && !fNoInsert && cmd->proc == getCommand && !vecInserts.empty()) { return;
robj *o = std::get<1>(vecInserts[0]);
if (o != nullptr) {
addReplyBulk(c, o);
return true;
}
}
return false;
} }

View File

@ -2595,9 +2595,7 @@ void parseClientCommandBuffer(client *c) {
(g_pserver->m_pstorageFactory || aeLockContested(cserver.cthreads/2) || cserver.cthreads == 1) && !GlobalLocksAcquired()) { (g_pserver->m_pstorageFactory || aeLockContested(cserver.cthreads/2) || cserver.cthreads == 1) && !GlobalLocksAcquired()) {
auto &query = c->vecqueuedcmd.back(); auto &query = c->vecqueuedcmd.back();
if (query.argc > 0 && query.argc == query.argcMax) { if (query.argc > 0 && query.argc == query.argcMax) {
if (c->db->prefetchKeysAsync(c, query, c->vecqueuedcmd.size() == 1)) { c->db->prefetchKeysAsync(c, query);
c->vecqueuedcmd.erase(c->vecqueuedcmd.begin());
}
} }
} }
c->reqtype = 0; c->reqtype = 0;

View File

@ -1204,7 +1204,7 @@ public:
void disableKeyCache(); void disableKeyCache();
bool keycacheIsEnabled(); bool keycacheIsEnabled();
bool prefetchKeysAsync(client *c, struct parsed_command &command, bool fExecOK); void prefetchKeysAsync(client *c, struct parsed_command &command);
bool FSnapshot() const { return m_spdbSnapshotHOLDER != nullptr; } bool FSnapshot() const { return m_spdbSnapshotHOLDER != nullptr; }