concurrency: stale Mutex.Unlock retry can delete a newer same-session lock

Bug report criteria

• This bug report is not security related, security issues should be disclosed privately via [email protected].
• This is not a support request or question, support requests or questions should be raised in the etcd discussion forums.
• I have read the etcd bug reporting guidelines.
• Existing open issues and the etcd frequently asked questions have been checked and this is not a duplicate.

What happened?

concurrency.Mutex.Unlock deletes the mutex key by key name only. If etcd commits the delete but the client observes an error, the old Mutex value retains its myKey and myRev. A later Mutex using the same session and prefix reuses the same key string because the key is derived from the session lease. The recreated key has a newer create revision. Retrying Unlock on the old Mutex then deletes that newer key incarnation.

This violates mutex ownership: an old unlock retry can release a later acquisition it does not own.

What did you expect to happen?

Unlock should not delete a key incarnation whose create revision differs from the revision recorded by that Mutex. The delete should be fenced by CreateRevision(m.myKey) == m.myRev, for example through the existing m.IsOwner() comparison.

How can we reproduce it (as minimally and precisely as possible)?

Add this integration test to tests/integration/clientv3/concurrency/mutex_test.go and run:

cd tests
go test ./integration/clientv3/concurrency -run '^TestMutexStaleUnlockDoesNotDeleteSameSessionRelock$' -count=1

Add these imports to the existing import block:

"context"
"sync/atomic"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"

func TestMutexStaleUnlockDoesNotDeleteSameSessionRelock(t *testing.T) {
	var loseFirstDeleteReply atomic.Bool
	loseFirstDeleteReply.Store(true)
	loseDeleteReply := func(
		ctx context.Context,
		method string,
		req, reply any,
		cc *grpc.ClientConn,
		invoker grpc.UnaryInvoker,
		opts ...grpc.CallOption,
	) error {
		err := invoker(ctx, method, req, reply, cc, opts...)
		deletesKey := method == "/etcdserverpb.KV/DeleteRange"
		if txn, ok := req.(*pb.TxnRequest); ok {
			for _, op := range txn.Success {
				deletesKey = deletesKey || op.GetRequestDeleteRange() != nil
			}
		}
		if err == nil && deletesKey && loseFirstDeleteReply.CompareAndSwap(true, false) {
			return status.Error(codes.DeadlineExceeded, "simulated lost delete response")
		}
		return err
	}

	cli, err := integration.NewClient(t, clientv3.Config{
		Endpoints:       exampleEndpoints(),
		MaxUnaryRetries: 1,
		DialOptions: []grpc.DialOption{
			grpc.WithChainUnaryInterceptor(loseDeleteReply),
		},
	})
	require.NoError(t, err)
	defer cli.Close()

	session, err := concurrency.NewSession(cli)
	require.NoError(t, err)
	defer session.Close()

	ctx := context.Background()
	oldMutex := concurrency.NewMutex(session, "/my-lock/")
	require.NoError(t, oldMutex.Lock(ctx))
	oldKey := oldMutex.Key()
	oldGet, err := cli.Get(ctx, oldKey)
	require.NoError(t, err)
	require.Len(t, oldGet.Kvs, 1)
	oldRevision := oldGet.Kvs[0].CreateRevision

	require.Error(t, oldMutex.Unlock(ctx))
	afterLostReply, err := cli.Get(ctx, oldKey)
	require.NoError(t, err)
	require.Empty(t, afterLostReply.Kvs)

	currentMutex := concurrency.NewMutex(session, "/my-lock/")
	require.NoError(t, currentMutex.Lock(ctx))
	require.Equal(t, oldKey, currentMutex.Key())
	currentGet, err := cli.Get(ctx, currentMutex.Key())
	require.NoError(t, err)
	require.Len(t, currentGet.Kvs, 1)
	require.Greater(t, currentGet.Kvs[0].CreateRevision, oldRevision)

	require.NoError(t, oldMutex.Unlock(ctx))
	afterStaleUnlock, err := cli.Get(ctx, currentMutex.Key())
	require.NoError(t, err)
	require.Len(t, afterStaleUnlock.Kvs, 1)
}

Anything else we need to know?

At the tested commit, Unlock calls an unconditional key-only delete, while IsOwner already exposes the recorded create-revision comparison.

One possible fix is to make Unlock a conditional transaction:

client.Txn(ctx).If(m.IsOwner()).Then(v3.OpDelete(m.myKey)).Commit()

The important part is that the delete is guarded by the create revision this Mutex recorded when it acquired the lock. The behavior when that comparison fails can be chosen separately; it must not delete the newer key.

Etcd version (please run commands below)

Confirmed on:

$ git rev-parse HEAD
12caed621b38312cc1084113415bf135c59e6457

$ go version
go version go1.26.5 linux/amd64

embedded integration server version: 3.8.0-alpha.0
also reproduced at v3.6.13 (b0f9ef190952e6e66a778513097a02ee41220727)

Etcd configuration (command line flags or environment variables)

Single-node embedded integration cluster from the etcd test framework.

Etcd debug information

No external cluster or separately installed etcdctl was used. The reproduction builds and starts an embedded server from the source checkout.

Relevant log output

--- FAIL: TestMutexStaleUnlockDoesNotDeleteSameSessionRelock
    Error: "[]" should have 1 item(s), but has 0
FAIL go.etcd.io/etcd/tests/v3/integration/clientv3/concurrency