Merge pull request #3 from 2-towns/fix/signal-closing

fix: never close the mapping signal from libplum's C thread
This commit is contained in:
Arnaud 2026-06-05 10:06:20 +04:00 committed by GitHub
commit 4e4d752a81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -88,8 +88,6 @@ type
resolvedInternalPort: uint16 resolvedInternalPort: uint16
resolvedExternalPort: uint16 resolvedExternalPort: uint16
resolvedExternalHost: array[PLUM_MAX_HOST_LEN, char] resolvedExternalHost: array[PLUM_MAX_HOST_LEN, char]
# Refcount-like
signalReleases: Atomic[int]
onStateChange: PlumStateCallback onStateChange: PlumStateCallback
# libplum calls mappingCallback from its own C thread. Under refc, any thread # libplum calls mappingCallback from its own C thread. Under refc, any thread
@ -108,10 +106,6 @@ var activeMappings {.guard: activeMappingsLock.}: Table[cint, MappingHandle]
initLock(activeMappingsLock) initLock(activeMappingsLock)
proc releaseSignal(handle: MappingHandle) =
if handle.signalReleases.fetchAdd(1) == 1:
discard handle.signal.close()
# We can be confident that the pattern is GC Safe using # We can be confident that the pattern is GC Safe using
# a lock. # a lock.
template withSafeLock(body: untyped) = template withSafeLock(body: untyped) =
@ -131,7 +125,9 @@ proc mappingCallback(id: cint, state: plum_state_t, raw: ptr plum_mapping_t) {.c
if plumState == Destroyed: if plumState == Destroyed:
withSafeLock: withSafeLock:
activeMappings.del(id) activeMappings.del(id)
handle.releaseSignal() # Do not close the signal here: close() unregisters the fd from the
# calling thread's dispatcher, and this is libplum's C thread, not the
# chronos loop. createMapping decides what happens to the signal.
# Release the pin set in createMapping: the C library is done with the # Release the pin set in createMapping: the C library is done with the
# raw pointer and will never call this callback again for this mapping. # raw pointer and will never call this callback again for this mapping.
GC_unref(handle) GC_unref(handle)
@ -236,11 +232,15 @@ proc createMapping*(
# CancelledError skips the lines below and propagates after finally. # CancelledError skips the lines below and propagates after finally.
raise raise
finally: finally:
if not completed: if completed:
# Timeout or cancellation: a late callback may still fireSync, so the # The signal fired (once, see mappingCallback) and was consumed:
# mapping is torn down and the close is decided by releaseSignal. # nobody will touch it again, safe to close here on the loop thread.
discard signal.close()
else:
# Timeout or cancellation: a late callback may still fireSync, so
# closing the signal is unsafe (here or from the C thread). We
# deliberately leak one eventfd instead.
discard plum_destroy_mapping(id) discard plum_destroy_mapping(id)
handle.releaseSignal()
# Timeout path: the signal never fired within the deadline. # Timeout path: the signal never fired within the deadline.
if not completed: if not completed: