Skip to content

Conversation

whitslack
Copy link
Collaborator

@whitslack whitslack commented Aug 22, 2025

  • pyln-client has a resource leak in UnixSocket.__init__(…) in the fallback path that is executed when the RPC socket path is longer than can fit in a struct sockaddr_un. This was causing file descriptor exhaustion in pytests.
    dirfd = os.open(dirname, os.O_DIRECTORY | os.O_RDONLY)
  • pyln-testing has a resource leak in utils.env(…) when reading config.vars.
    lines = open(fname, 'r').readlines()
  • pyln-testing has a resource leak in node_factory, as it neglects to call cleanup_files() on the LightningD instances produced by the factory.
  • pyln-testing has a resource leak in GossipStore, as it neglects to close the underlying file descriptor when an instance is discarded or when an instance is re-opened.
  • Many locations throughout tests/ had file descriptor leaks due to calling open or pathlib.Path.open without using with and without calling close(). These are fixed using with or pathlib.Path.{read,write}_bytes.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed. Not applicable.
  • Related issues have been listed and linked, including any that this PR closes. None found.

This file descriptor leak was causing test failures due to exceeding the
limit on open file descriptors. Note that the leak only occurred if the
RPC socket path was longer than can fit in a struct sockaddr_un.

Changelog-Fixed: pyln-client no longer leaks a file descriptor when connecting to an RPC socket with a long path name.
This code has a resource leak:

	lines = open(fname, 'r').readlines()

This is the correct way:

	with open(fname, 'r') as f:
		lines = f.readlines()

Changelog-None
TailableProc has a cleanup_files() method to close its log files. Call
it when tearing down node_factory to avoid leaking resources.

Changelog-None
@whitslack whitslack requested a review from cdecker as a code owner August 22, 2025 18:21
@whitslack whitslack changed the title Fix three resource leaks Fix four resource leaks Aug 23, 2025
@whitslack
Copy link
Collaborator Author

It is possible (I didn't check) that the resource leak in node_factory predates #7669, which fixed the FD leak in TailableProc for BitcoinD but not for LightningD.

@whitslack whitslack changed the title Fix four resource leaks tests: Fix numerous resource leaks Aug 24, 2025
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack 24c9abb

@ShahanaFarooqui ShahanaFarooqui added this to the v25.12 milestone Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants