Using async wisely

[VerusK]

This PR addresses a critical issue encountered when scraping large websites with over 1 million pages. Previously, goroutines were being spawned without any limit, leading to significant memory bloat. This update introduces a more efficient management system for goroutines, effectively resolving the memory issues.

Regarding this issue #381

[StantonMatt]

I checked this against #381 because the goal makes sense: async collectors should not be allowed to queue unbounded goroutines behind the backend limiter.

The branch is auto-mergeable with current master, and these pass on head e34cc4c:

• go test ./... -count=1
• git diff --check origin/master...HEAD

There is a regression from moving the same LimitRule waitChan acquisition into scrape without removing or changing the backend acquisition. The new scrape-side acquisition also runs for synchronous collectors. With Async left at its default false and Parallelism set to 1, scrape fills r.waitChan and then calls fetch synchronously; fetch reaches httpBackend.Do, which tries to send to the same full waitChan before scrape can return and run its defer. That makes Visit block forever.

I verified this with a local-only test that starts a httptest server, creates a default NewCollector, adds LimitRule{DomainGlob: *, Parallelism: 1}, runs c.Visit(ts.URL) in a goroutine, and fails if it does not return within 500 ms. That test passes on current master and fails on this branch with synchronous Visit with LimitRule did not return.

There is also a small mechanical issue: gofmt -l colly.go reports colly.go on this branch.

I think the fix needs to avoid double-acquiring the same limiter. Either the new pre-fetch limiter should only apply to Async in a way that hands the slot to fetch, or the existing backend limiter needs to be refactored so synchronous visits and actual request execution still have one clear limiter owner.

[StantonMatt]

I checked this against the current merge ref (cdd4525). The direction matches #381, but I found a blocking regression in the current shape.

The branch adds a LimitRule wait in Collector.scrape, while httpBackend.Do still applies the same matching rule. That means synchronous collectors acquire the same waitChan twice. With Parallelism: 1, Visit does not return because scrape holds the slot while waiting for fetch, and fetch blocks trying to enter the backend limiter.

Local repro:

func TestPR797SyncLimitRuleDoesNotDeadlock(t *testing.T) {
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.WriteHeader(http.StatusOK)
    }))
    defer ts.Close()

    c := NewCollector()
    _ = c.Limit(&LimitRule{DomainGlob: "*", Parallelism: 1})

    done := make(chan error, 1)
    go func() { done <- c.Visit(ts.URL) }()

    select {
    case err := <-done:
        if err != nil {
            t.Fatal(err)
        }
    case <-time.After(500 * time.Millisecond):
        t.Fatal("Visit did not return with a synchronous collector and Parallelism=1")
    }
}

Result on the PR merge ref:

Visit did not return with a synchronous collector and Parallelism=1

The same temporary test passes on current master.

One smaller check: gofmt -l colly.go also reports colly.go on the merge ref, likely from the import grouping change.