Skip to content

Fix JSON.MSET duplicate-key crash#112

Merged
roshkhatri merged 1 commit into
valkey-io:unstablefrom
roshkhatri:fix-issue-106-mset-duplicate-key
Jun 11, 2026
Merged

Fix JSON.MSET duplicate-key crash#112
roshkhatri merged 1 commit into
valkey-io:unstablefrom
roshkhatri:fix-issue-106-mset-duplicate-key

Conversation

@roshkhatri

Copy link
Copy Markdown
Member

Fix for: #106

A JSON.MSET that references the same key more than once could crash the server
(SIGABRT). Triples are validated up front and then applied in place under
ValkeyModule_Assert(rc == JSONUTIL_SUCCESS). For a duplicated key, an earlier op changes
the document's shape, so a later op (validated against the original shape) fails at apply
time and the assertion aborts the process.

This removes the apply-phase assertions and applies ops on a best-effort basis. Each key
is opened fresh at apply time, so every op sees the current value, including the effect of
an earlier op on the same key. An op that cannot apply against the current document is
skipped and the remaining ops are still applied; the command returns OK. Up-front
validation is unchanged, so malformed commands are still rejected atomically with an error
before any write.

This also makes JSON.MSET behavior compatible with RedisJSON.

Tests: added cases for a duplicated key whose later op conflicts with the first (returns
OK, server alive, first op committed), a three-op case where a middle op is skipped and
the others applied, and a case where the later op is valid against the first op's result
so both apply. Existing MSET and validation tests pass unchanged; the full integration
suite passes across all server versions including the ASAN build.

Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
@roshkhatri roshkhatri self-assigned this Jun 10, 2026

@zackcam zackcam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, should we put a note in the docs that the behaviour now will silently fail on a bad duplicate key but still return ok?

@roshkhatri roshkhatri merged commit 0c320dd into valkey-io:unstable Jun 11, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants