diff --git a/internal/protocol/http/fetcher.go b/internal/protocol/http/fetcher.go index e7ba4b2f..cfc4f9f8 100644 --- a/internal/protocol/http/fetcher.go +++ b/internal/protocol/http/fetcher.go @@ -418,16 +418,6 @@ func (f *Fetcher) updateMaxConnTime(d time.Duration) { } func (f *Fetcher) Resolve(req *base.Request) error { - return f.ResolveContext(context.Background(), req) -} - -// ResolveContext cancels the initial request while waiting for headers. Once -// Resolve has returned, the retained response belongs to the fetcher and its -// existing Pause/Close lifecycle controls the body and prefetch. -func (f *Fetcher) ResolveContext(ctx context.Context, req *base.Request) error { - if err := ctx.Err(); err != nil { - return err - } if err := base.ParseReqExtra[fhttp.ReqExtra](req); err != nil { return err } @@ -439,8 +429,6 @@ func (f *Fetcher) ResolveContext(ctx context.Context, req *base.Request) error { // This allows the response to be reused for downloading (important for one-time URLs) // The body outlives Resolve, so the read timeout gets its own cancel func. reqCtx, reqCancel := context.WithCancel(context.Background()) - stopCancel := context.AfterFunc(ctx, reqCancel) - defer stopCancel() httpReq, err := f.buildRequest(reqCtx, req) if err != nil { reqCancel() diff --git a/pkg/download/downloader.go b/pkg/download/downloader.go index 6fcd0419..84d07aae 100644 --- a/pkg/download/downloader.go +++ b/pkg/download/downloader.go @@ -563,16 +563,9 @@ func ensureResourceRequestRawURLs(parentReq *base.Request, res *base.Resource) { // resolveState owns a Fetcher until Create transfers it to a task. // State transitions are protected by fetcherMapLock; closeOnce serializes cleanup. -type contextResolver interface { - ResolveContext(context.Context, *base.Request) error -} - type resolveState struct { - ctx context.Context - cancel context.CancelFunc fetcher fetcher.Fetcher cancelled bool - resolved bool claimed bool closeOnce sync.Once closeErr error @@ -602,9 +595,7 @@ func (d *Downloader) ResolveWithID(id string, req *base.Request) (rr *ResolveRes if req == nil { return nil, errors.New("resolve request is required") } - resolveCtx, cancel := context.WithCancel(d.backgroundCtx) - defer cancel() - state := &resolveState{ctx: resolveCtx, cancel: cancel} + state := &resolveState{} d.fetcherMapLock.Lock() if d.closed.Load() { d.fetcherMapLock.Unlock() @@ -645,11 +636,7 @@ func (d *Downloader) ResolveWithID(id string, req *base.Request) (rr *ResolveRes if cancelled { return nil, errors.New("resource resolution cancelled") } - if resolver, ok := f.(contextResolver); ok { - err = resolver.ResolveContext(state.ctx, req) - } else { - err = f.Resolve(req) - } + err = f.Resolve(req) if err != nil { return nil, err } @@ -658,13 +645,13 @@ func (d *Downloader) ResolveWithID(id string, req *base.Request) (rr *ResolveRes if state.cancelled || d.closed.Load() { return nil, errors.New("resource resolution cancelled") } - state.resolved = true d.fetcherCache[id] = f return &ResolveResult{ID: id, Res: f.Meta().Res, SuggestedDownloadDir: d.suggestedDownloadDir(f.Meta().Res)}, nil } // ReleaseResolve is idempotent. A resource already transferred to a task is // never closed by a late preview cancellation. +// Pending resolution is interrupted according to the Fetcher's Close semantics. func (d *Downloader) ReleaseResolve(id string) error { d.fetcherMapLock.Lock() state := d.resolveStates[id] @@ -673,17 +660,10 @@ func (d *Downloader) ReleaseResolve(id string) error { delete(d.resolveStates, id) if state != nil { state.cancelled = true - state.cancel() if state.claimed { f = nil } else { f = state.fetcher - if _, cancellable := f.(contextResolver); cancellable && !state.resolved { - // Resolve owns unpublished response/prefetch state. Its context - // cancels the request; the returning resolver closes it once - // publication has stopped, including a racing response. - f = nil - } } } d.fetcherMapLock.Unlock() diff --git a/pkg/download/resolve_lifecycle_test.go b/pkg/download/resolve_lifecycle_test.go index 553738b2..8f1d6371 100644 --- a/pkg/download/resolve_lifecycle_test.go +++ b/pkg/download/resolve_lifecycle_test.go @@ -3,50 +3,15 @@ package download import ( "errors" "fmt" - "github.com/GopeedLab/gopeed/internal/fetcher" "net/http" "net/http/httptest" "testing" "time" + "github.com/GopeedLab/gopeed/internal/fetcher" "github.com/GopeedLab/gopeed/pkg/base" ) -func TestReleaseResolveCancelsPendingHTTPHeaders(t *testing.T) { - entered := make(chan struct{}) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { close(entered); <-r.Context().Done() })) - defer server.Close() - d := NewDownloader(&DownloaderConfig{Storage: NewMemStorage(), StorageDir: t.TempDir(), TempDir: t.TempDir()}) - if err := d.Setup(); err != nil { - t.Fatal(err) - } - defer d.Close() - done := make(chan error, 1) - go func() { - _, err := d.ResolveWithID("pending-http", &base.Request{URL: server.URL + "/file"}) - done <- err - }() - select { - case <-entered: - case <-time.After(time.Second): - t.Fatal("request did not start") - } - if err := d.ReleaseResolve("pending-http"); err != nil { - t.Fatal(err) - } - select { - case err := <-done: - if err == nil { - t.Fatal("cancelled Resolve succeeded") - } - case <-time.After(300 * time.Millisecond): - server.CloseClientConnections() - t.Fatal("ReleaseResolve did not cancel the request waiting for headers") - } - if len(d.fetcherCache) != 0 || len(d.resolveStates) != 0 { - t.Fatal("cancelled operation retained resources") - } -} func TestReleaseResolveAfterCreateDoesNotCloseTask(t *testing.T) { d := NewDownloader(&DownloaderConfig{Storage: NewMemStorage(), StorageDir: t.TempDir(), TempDir: t.TempDir(), FetchManagers: nil}) if err := d.Setup(); err != nil {