Prefix::size_ counts bits, but commonBits walked content_ a byte at a
time up to it. Two prefixes that agree on their whole common length are
therefore read well past their end, which AddressSanitizer reports as a
heap-buffer-overflow, and the count that comes back is eight times too
large. A lookup with a common prefix length hits this as soon as an
index entry matches the key it is looking for, and entry.prefix comes
straight off the network. The flags were indexed the same way, so a
partially known prefix was read at the wrong position too.
swapBit had the other half of the confusion: it flipped bit
(8 - bit) % 8 of the byte, while isActiveBit reads bit 7 - (bit % 8).
getSibling, which flips the last bit of a prefix to name the sibling
node in the trie, therefore returned a prefix differing in some other
bit and sharing the last one, so lookup and insertion looked up the
wrong node. Splitting a leaf and finding a canary both rely on it.
Add tests covering both, which fail before the fix: commonBits returns
193 for two identical 160 bit prefixes, and a prefix and its sibling
share 7 bits rather than all but the last.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve idle-runner push futures immediately under the operation queue lock. A stopped runner has no loop left to execute the queued promise, otherwise consumers can wait indefinitely.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require immediate completion before run and after join, including a burst of notifications. Keep active-runner coverage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ValueCache re-emitted a known value as an edition on a higher seq alone,
while OpValueCache requires the content to differ as well. Since
Value::operator== ignores seq, identical content re-announced with a
bumped seq was re-emitted as an add: a source-counting OpValueCache
then counted the same node twice and kept the value after the node
expired or left the search. Apply the same rule at both levels.
Value::operator== ignores seq, so a node re-announcing identical content
with a bumped seq is the same value refreshed, not an edition. ValueCache
must not re-emit it: a source-counting OpValueCache above would count the
node twice and keep the value after the node is gone.
onValuesAdded ignores a different payload that does not carry a higher
seq (e.g. the same signed content re-signed with a randomized
signature), but onValuesExpired only skipped strictly older versions.
The uncounted source could therefore decrement the refcount and expire
a value still held by others. Apply the same rule on both sides.
A source reporting a different payload with the same seq is ignored on
add and not counted; its later expiration must not release the value
still held by the counted sources.
~DhtProxyServer destroyed requests_ whose ~http::Request notifies DONE;
the handler registered by sendPushNotification then erased from the map
being destroyed, corrupting the heap (double free under ASan) whenever a
push request was still pending, e.g. an unreachable push server. Detach
the state-change callbacks and release the requests before the members
are destroyed.
Storage::store reported edited_values=1 whenever the stored Sp<Value>
differed from the incoming one, even with equal content. A local put()
of an already-stored value was therefore announced to local listeners
as a new value a second time, unbalancing listen caches that count how
many sources hold a value. Only report an edition when the content
actually changed; the expiration is still refreshed.
SearchCache::get called the GetCallback unconditionally; Dht::query
passes none, which threw bad_function_call as soon as a listen cache
existed for the key. Return false without a callback so the network
path runs. A callback returning false means the consumer has enough
values, not a failure: report done(true) as the network path does.
DhtProxyClient push refresh expired every previously cached value once
the refresh get completed, including values that were just re-delivered
or edited, producing phantom expire callbacks. Track the ids received by
the get and only expire cached values missing from it, and skip the
reconciliation when the get failed so a transient error does not expire
values still present on the DHT.
ValueCache emitted any content change of a known value as an edition
while OpValueCache only accepts a higher seq. A node reporting an older
version made ValueCache replace its copy and re-emit versions that were
then dropped or, with source counting, double counted downstream. Use
the same rule at both levels.
OpValueCache counted every re-delivery of an identical value as a new
source, but the proxy client is a single logical source that re-sends
values on push refresh or listen restart: the refcount grew unbounded
and expirations were never propagated. Add a countSources flag to
OpValueCache, OpCache and SearchCache; the proxy client disables it.
When a value is edited (same id, higher seq), only the reporting source
is known to hold the new version: replace it in place, reset the
refcount to 1 and emit a single add. Ignore older versions from lagging
sources on add, and ignore their expiration so they cannot release a
newer version still held by others.
ValueCache emits an edited value as an add instead of silently
refreshing its old copy, and applies a grace period of one type
expiration while the listen is synced so that a single lost refresh
notification does not drop a value the remote node still holds.
SearchCache::get only answers from the cache once the listen is synced,
so a get never returns a partial view that the network would complete.
Add the single-source OpCacheTester cases enabled by countSources.
OpCacheTester and ValueCacheTester exercise the listen caches in
isolation: add/edit/expire flows, per-source refcounting, stale
versions, sync status, SearchCache::get and node drop.
DhtRunnerTester and DhtProxyTester gain end-to-end listen tests for
value edition, duplicate puts, multi-source and partial expiration,
get after a synced listen, and the push refresh/expire paths.
The proxy fixture joins the proxy DhtRunner before destroying the
DhtProxyServer so no DHT callback can reach a destroyed server, and
push notifications are never awaited while holding the mutex taken by
listen callbacks: the result is produced on the DHT thread.
Register the new executables in CMake and Meson.
asio sockets are not thread-safe. http::Connection::close() used to close
the socket synchronously on the calling thread. Request::cancel() and
Request::~Request() (via terminate()) are called from arbitrary threads
(e.g. DhtProxyClient::stop(), cancelAllListeners(), cancelListen()),
so the close raced with in-flight operations on the io_context thread.
In particular, asio::async_connect's range_connect_op closes and
re-opens the socket when trying the next endpoint (common on mobile
where the IPv6 endpoint fails first). A concurrent close() from another
thread corrupts the epoll reactor's per-descriptor state, leading to
crashes in epoll_reactor::deregister_descriptor / register_descriptor
called from range_connect_op::process.
Dispatch the close to the io_context thread when not already on it.
Also keep the connection alive during the TLS handshake, like every
other asynchronous operation, so that the destructor can never run
while an operation is pending.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adrien Beraud
Cyrille BéraudandCopilot
copilot-swe-agent[bot]andaberaud
dependabot[bot]