Skip to content

getFileLock in files/public.go leaks memory via unbounded fileLocks map #1570

Closed

Description

@jmunson

The LinkFromPool concurrency fix (commit 3608c13 / PR #1449) introduced a per-path mutex map to prevent race conditions during concurrent publish operations:

    var (
        fileLockMutex sync.Mutex
        fileLocks = make(map[string]*sync.Mutex)
    )

    func getFileLock(filePath string) *sync.Mutex {
        fileLockMutex.Lock()
        defer fileLockMutex.Unlock()

        if mutex, exists := fileLocks[filePath]; exists {
            return mutex
        }
        mutex := &sync.Mutex{}
        fileLocks[filePath] = mutex
        return mutex
    }

The problem is that fileLocks is append-only. Every unique destinationPath ever passed to LinkFromPool allocates a *sync.Mutex entry that is never removed. This is a slow but unbounded memory leak - estimated impact is around 150-200 bytes per unique path, so after 100,000 packages are registered I'd expect around ~20MB of extra memory used.

That's small enough that it might be worth just accepting as a tradeoff as attempting to prevent this definitely adds a lot of complexity - either adding a refcount to fileLocks, or storing these in a Least Recently Used list where we can bound the size of filelocks and once we hit that capacty we start evicting the oldest ones.

Activity

  1. self-assigned this
    on May 31, 2026
  2. added this to the 1.6.3 milestone on May 31, 2026
  3. neolynx commented on May 31, 2026

    @neolynx
    Member

    Thanks for finding this !

    I am having a close look at all the race condition fixes from the past. And this file lock might actually not be needed and hide underlying issues.

    @agustinhenze: I am still analyzing this in the context of #1511, what do you think about the file lock fix, woudl that still be needed ?

    maybe we can remove it, and the mem leak would be gone as well :)

  4. agustinhenze commented on Jun 3, 2026

    @agustinhenze
    Contributor

    Mm right, not sure why we haven't seen that before 😬. @neolynx not following all the changes you have done so far, but as long as the test case pass I think is ok. The test case written was hitting the issue very soon (first iterations) without the mutex and in my setup almost everyday.

  5. neolynx commented on Jun 7, 2026

    @neolynx
    Member

    I have reverted the filelock, and added the tests: #1574

    Now, pool is now locked on MultiDist=false, therefore the race condition is gone, and file lock is not needed. No more mem leaks..

    @jmunson @agustinhenze could you give this a try: https://github.com/aptly-dev/aptly/actions/runs/27106079209?pr=1574 ?

    thanks a lot 👍

  6. neolynx commented on Jun 11, 2026

    @neolynx
    Member
  7. added
    please confirm resolvedWe believe the issue is resolved ! if so, please close the issue, thanks ;-)
    on Jun 13, 2026
  8. neolynx commented on Jun 19, 2026

    @neolynx
    Member

    Release 1.6.3 coming soon, please try latest CI build to confirm the issue is solved:
    #1575 (comment)

  9. neolynx commented on Jun 25, 2026

    @neolynx
    Member

    We believe this has been addressed in the 1.6.3 release: #1597 ! :-)

    Please reopen if the problem persists.

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

Metadata

Metadata

Assignees

Labels

bugplease confirm resolvedWe believe the issue is resolved ! if so, please close the issue, thanks ;-)

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions