-
-
Notifications
You must be signed in to change notification settings - Fork 425
Description
Is there an existing issue for this?
- There is no existing issue for this bug
Is this happening on an up to date version of Incus?
- This is happening on a supported version of Incus
Incus system details
latest master branchInstance details
No response
Instance log
No response
Current behavior
Small issue found in the code, I'm not sure how feasible it is to trigger it, but it looks obviously wrong to me, so I'm reporting it.
UpdateMetadata and ExtendMetadata in internal/server/operations/operations.go can leak op.lock if parseMetadata returns an error. The lock is acquired before the call but the error return path doesn't release it.
All the other error paths do release it.
UpdateMetadata (lines 585–599):
func (op *Operation) UpdateMetadata(opMetadata any) error {
op.lock.Lock() // lock acquired
// ... status/readonly checks with Unlock ...
newMetadata, err := parseMetadata(opMetadata)
if err != nil {
return err // ← returns without Unlock
}ExtendMetadata (lines 619–635):
func (op *Operation) ExtendMetadata(metadata any) error {
op.lock.Lock() // lock acquired
// ... status/readonly checks with Unlock ...
extraMetadata, err := parseMetadata(metadata)
if err != nil {
return err // ← returns without Unlock
}How it was introduced:
Commit ad73f87a6 ("lxd/operations: Fix possible races on accessing status property", May 2022) moved op.lock.Lock() above the status and readonly checks so that reading op.status and op.readonly would be protected by the lock. This was correct, but as a side-effect the parseMetadata() call — which previously ran before the lock was acquired — ended up inside the locked section. The commit added op.lock.Unlock() for the status/readonly early-return paths but missed the parseMetadata error return path.
PS: this was found by an LLM while doing an unrelated investigation. Reporting because it sounds obviously buggy to me, to the best of my knowledge.
Expected behavior
No response
Steps to reproduce
.