Skip to content

[Feature] Normalize cached link ownership and resource lifetime #3098

Description

@nostalume

Please confirm the following

  • I have read and agree to AGPL-3.0 Section 15. The program is provided "as is" without any warranties; you bear all risks of using it.
  • I have read and agree to AGPL-3.0 Section 16. The copyright holders and distributors are not liable for any damages resulting from the use or inability to use the program.
  • I confirm my description is clear, polite, helps developers quickly locate the issue, and complies with community rules.
  • I have read the OpenList documentation.
  • I confirm there are no duplicate issues or discussions.
  • I believe this issue must be handled by OpenList and not by a third party.
  • I confirm this feature has not been implemented yet.
  • I confirm this feature is reasonable and has general demand, not just my personal need.
  • I have not read these checkboxes and therefore I just ticked them all, Please close this issue.

Feature Description

OpenList's download path currently represents several different responsibilities with model.Link:

  • a URL or RangeReader transport description;
  • request headers, content length, and partitioning hints;
  • cache expiration policy;
  • shared SyncClosers reference state; and
  • the close handle returned to one caller.

The same value is constructed by drivers, cached and reference-acquired by internal/op, normalized by internal/fs, converted to readers by internal/stream, and finally adopted by HTTP, WebDAV, S3, FTP, MCP, transfer tasks, and wrapper drivers. Ordinary links and archive extraction currently repeat the cache-expiration, reference-acquisition, and retry protocol. GetRangeReaderFromLink returns the reader capability separately from the link whose resources keep it valid, leaving adapters to reconstruct close ownership.

This has a large compatibility surface: model.Link appears in 129 Go files and 89 driver methods implement Link. It should therefore be treated as a multi-PR migration, not a single type replacement. Recent focused fixes illustrate the boundary risk: #3090 preserves cancellation failures in partitioned downloads, and #3095 makes S3 ranged response bodies own both the opened reader and link reference. #3082's SFTP throughput investigation is a related performance constraint that the migration must not regress.

The goal is one explicit owner for each acquired download resource, while preserving current routes, protocol responses, driver behavior, cache keys and expiration, cancellation, and redirect/proxy policy. This issue does not propose a user-facing API or configuration change.

Suggested Solution

Use an incremental roadmap behind the existing driver interface:

  1. Characterize direct/partitioned URL and RangeReader paths, expiring and reference-required cache reuse, archive extraction, redirects, cancellation, and close order through observable tests.
  2. Remove internal application packages' dependency on server/common for request-origin calculation. Admit request origin once at the delivery boundary and preserve generated URL behavior.
  3. Introduce one internal acquired-link lifetime behind op.Link without changing all drivers at once. A migrated caller must have exactly one close obligation; do not retain parallel old/new ownership conventions.
  4. Make ordinary link caching and archive extraction share the acquisition, expiration, reference, and retry policy, then delete the duplicate protocol.
  5. Make range-reader adoption return a resource that owns both the opened body and acquired link in the required close order. Migrate HTTP, WebDAV, S3, FTP, MCP, transfer tasks, and wrapper drivers as separate reviewable slices.
  6. After every consumer migrates, reassess and remove cache/lifetime fields from model.Link only where no compatibility consumer remains.

Each implementation PR should be independently revertible and must include focused behavior, failure/cancellation, close-count, and Linux race evidence. Driver-facing interfaces should remain compatible until the final migration gate. Performance validation must include the SFTP/high-latency case from #3082 where applicable.

We plan to submit future PRs in this dependency order rather than combine the migration into one large diff. The design should be reopened if a driver requires a genuinely different lifetime, if an adapter cannot adopt the common acquired resource without changing protocol behavior, or if the shared acquisition policy regresses cache freshness, cancellation, redirect policy, or throughput.

Relationship to current work

These are behavioral and verification relationships, not a stacked-PR chain. Roadmap implementation PRs should branch from refreshed main after the relevant predecessors are settled, remain independently revertible, and name any contract evidence they reuse.

Delivery policy: item 1's characterization evidence travels with the first PR that changes its owning boundary rather than becoming a standalone test PR. Item 2 may be developed on an independent, main-based fork branch, but should be submitted upstream only if its actual diff removes all eight origin-only internal → server/common dependencies without adding a package, facade, configuration field, or behavior change. Otherwise retain the incumbent or fold the boundary change into the first lifecycle PR that genuinely needs it. Dependent later work may proceed on local child branches or worktrees, but cross-fork branches are not proposed as a public stacked-review chain.

Additional Information

Current module evidence at main@4580c4db:

  • internal/model: 1,032 production lines, imported by 371 Go files, no package tests;
  • internal/op: 3,163 production lines, imported by 244 Go files;
  • internal/fs: 1,180 production lines, imported by 33 Go files;
  • model.Link: referenced by 129 Go files;
  • op.Link: 14 call sites; fs.Link: 11 call sites; GetRangeReaderFromLink: 11 call sites;
  • eleven internal files import packages under server/, including five internal/fs files.
  • acquisition analysis identifies TTL-shared, reference-shared, and close-invalidated legacy modes. Follow-up tracing found the conflicting state is reachable when Link.Clone copies TTL while owning its source; fix(op): enforce link cache lifecycle policy #3101 makes internal/op the single admission owner, rejects TTL plus owned resources, and stops clones from propagating cache policy. The acquired-resource carrier remains a later decision requiring wrapper-transfer and close-order evidence.
  • item 2 is published as refactor(context): centralize request origin access #3100: it removes all eight origin-only internal dependencies on server/common, retains a narrow compatibility bridge for existing server/driver consumers, and passes focused behavior and Linux race checks.

Repository issue searches for link lifecycle, RangeReader close, link cache reference closer, and server common dependency found no existing roadmap. #3082 is related to SFTP streaming performance but does not cover ownership migration.

AI Generated Content

  • I used AI tools to generate this content
  • I did not use AI tools to generate this content

AI model used

Codex (GPT-5)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions