Skip to content

HDDS-15357. Fix MappedBufferManager WeakReference races#10351

Draft
smengcl wants to merge 1 commit into
apache:masterfrom
smengcl:mapped-buffer-weakref-fix
Draft

HDDS-15357. Fix MappedBufferManager WeakReference races#10351
smengcl wants to merge 1 commit into
apache:masterfrom
smengcl:mapped-buffer-weakref-fix

Conversation

@smengcl
Copy link
Copy Markdown
Contributor

@smengcl smengcl commented May 24, 2026

What changes were proposed in this pull request?

MappedBufferManager caches mapped buffers as WeakReference<ByteBuffer> and tracks in-flight quota via a Semaphore. Two races on master:

  1. computeIfAbsent GC race: The cache-hit branch reads refer.get() once for a null check and a second time at the return. Between the two reads, the WeakReference is the only handle to the buffer. GC may clear it, the second read returns null, and ChunkUtils.readData then dereferences a null IOException (throw null).

  2. Cleanup-loop races in getQuota: The async loop walks mappedBuffers.keySet() and re-looks up each key. A concurrent remove() makes the lookup return null and the chained .get() NPEs. A concurrent put() that replaces the WeakReference makes the unconditional mappedBuffers.remove(key) drop someone else's fresh entry. p is incremented for it, and releaseQuota(p) over-releases the semaphore.

Fix

  • computeIfAbsent resolves refer.get() once into a local final ByteBuffer cached and returns that. The stack-local strong reference keeps the buffer reachable until the method returns.
  • The cleanup loop iterates entries directly (no key-then-get re-lookup, no NPE) and uses ConcurrentHashMap.remove(key, value) so a concurrent put() that replaced the WeakReference returns false from the conditional remove and is not counted as freed quota.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15357

How was this patch tested?

  • Existing tests

@smengcl smengcl added the bug Something isn't working label May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant