fix(amuleweb): clear search results across cycles + on new search (#31) - #33
Merged
got3nks merged 2 commits intoJun 9, 2026
Merged
Conversation
Commit ee1d92b wired the partial-update protocol's skip-unchanged + EC_TAG_FILE_REMOVED logic into the daemon's shared-files and download-queue handlers, and amuleweb's shared `UpdatableItemsContainer ::ProcessUpdate` learned to expect explicit deletion markers whenever the negotiated capability was active. `SearchInfo` inherits the same `ProcessUpdate` template body, but `Get_EC_Response_Search_Results` was never extended to emit those markers. The result is that on any 3.0.0 amuleweb against a 3.0.0 amuled, search results monotonically accumulate: a new search clears the daemon's searchlist but the next `EC_OP_SEARCH_RESULTS` reply just lists the new search's items with no tombstones for the old ones, so the client never drops them (amule-project#31). Add per-session `m_lastSentSearchIds` to CECServerSocket, snapshot the current search-result ECIDs on each poll, emit one EC_TAG_FILE_REMOVED per ID that was in the previous reply but is no longer in the searchlist, then swap the snapshot in. Gate symmetrically with the shared/download handlers — only `EC_DETAIL_UPDATE` polling from a partial-update-capable client, and only when no `queryitems` subset is specified, so amulegui's `EC_DETAIL_INC_UPDATE` overload, amulecmd's `EC_DETAIL_FULL` one-shots, and amuleweb's Phase-3 follow-up (`req_full`, defaults to `EC_DETAIL_FULL`) are untouched. Pure server-side fix; no client change needed: amuleweb's existing `UpdatableItemsContainer::ProcessUpdate` already drains `EC_TAG_FILE_REMOVED` from the reply.
got3nks
marked this pull request as draft
June 9, 2026 08:53
…ect#31) SearchFile's constructor never forwards `tag->ID()` to its `CECID` base, so every instance gets a fresh local counter from `CECID::CECID()` instead of the daemon's ECID. Sibling SharedFile / DownloadFile / UploadFile all do this correctly: SharedFile::SharedFile(CEC_SharedFile_Tag *tag) : CECID(tag->ID()) SearchFile alone was left with the default constructor. Consequence: amuleweb's `m_items_hash` (keyed by SearchFile::ID() = local counter) can never match `tag->ID()` (= daemon ECID) on incoming updates. Every `UpdatableItemsContainer::ProcessUpdate` cycle treats every result as new, queues every ID into the Phase-3 follow-up, and `ProcessFull` appends each item again -- visible to the user as the result list growing on every "Update results" click. Latent since the file was written: the bulk "missing-from-reply == deleted" loop in pre-partial-update ProcessUpdate happened to wipe the list every cycle, so duplicates got rebuilt rather than accumulating. ee1d92b (EC skip-unchanged 5/5) made that loop conditional on `m_partialUpdateActive`, and the bug surfaced. Also a prerequisite for the EC_TAG_FILE_REMOVED tombstoning in this PR's previous commit: tombstones carry daemon ECIDs; with the local counter, `m_items_hash.erase(daemon_ecid)` would have never matched anything.
got3nks
changed the title
got3nks
marked this pull request as ready for review
June 9, 2026 09:34
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #31 — amuleweb's search results grow on every "Update results" click and don't clear when a new search starts. Two independent bugs, both contributing:
Bug 1 —
SearchFileECID was never forwarded from the tag (latent forever)SearchFile(CEC_SearchFile_Tag *tag)never calls: CECID(tag->ID()). The defaultCECID()constructor auto-increments a static counter, so everySearchFileinstance ends up with a local counter as its ID — completely disconnected from the daemon's ECID the tag carries. SiblingSharedFile/DownloadFile/UploadFileall forward correctly;SearchFilealone was left with the default ctor.Consequence:
m_items_hash.count(tag->ID())always returns 0, every poll treats every result as new,ProcessFullre-appends each item — visible to the user as the result list growing on every "Update results" click.Latent since the file was written. The pre-partial-update bulk "missing-from-reply == deleted" loop happened to wipe the list every cycle, so duplicates got rebuilt rather than accumulating. ee1d92b (
EC skip-unchanged 5/5) made that loop conditional onm_partialUpdateActive, and the bug surfaced.Bug 2 — daemon never emitted
EC_TAG_FILE_REMOVEDtombstones for search resultsGet_EC_Response_GetSharedFilesandGet_EC_Response_GetDownloadQueuegot partial-update tombstoning in ee1d92b.Get_EC_Response_Search_Resultswas missed. Once partial-update is negotiated, amuleweb'sUpdatableItemsContainer::ProcessUpdateexpects explicitEC_TAG_FILE_REMOVEDmarkers and skips its legacy bulk-delete; without them, items from previous searches linger forever.Bug 1 is a prerequisite for Bug 2's fix: tombstones carry daemon ECIDs, and without the
SearchFileECID forwarding,m_items_hash.erase(ecid)would have never matched.Fix
Two commits:
fix(ec): emit EC_TAG_FILE_REMOVED tombstones on EC_OP_SEARCH_RESULTS— daemon-side. New per-sessionm_lastSentSearchIdsonCECServerSocket; snapshot current search-result ECIDs each poll, emit one tombstone per ID that disappeared. Gated identically to the shared/download handlers (m_partialUpdateActive && detail_level == EC_DETAIL_UPDATE && queryitems.empty()).fix(amuleweb): initialize SearchFile ECID from daemon tag— client-side. One line:SearchFile::SearchFile(...) : CECID(tag->ID()).Backward compatibility
No protocol change.
The tombstone gate skips amulegui (different
EC_DETAIL_LEVEL→ different daemon overload), amulecmd (EC_DETAIL_FULL/EC_DETAIL_CMD), and amuleweb's Phase-3 follow-up (EC_DETAIL_FULLdefault from the single-argCECPacketctor). Old amuleweb against new daemon is unchanged — old client doesn't negotiate the protocol → gate off.The SearchFile ctor fix is a pure client-side correction with no protocol effect.
Test plan
amuledfrom this branch on a Linux amule-dev VM