Skip to content

Fix JGit resources not being closed in CLI commands#6795

Merged
pditommaso merged 4 commits intomasterfrom
fix/close-jgit-resources
Feb 11, 2026
Merged

Fix JGit resources not being closed in CLI commands#6795
pditommaso merged 4 commits intomasterfrom
fix/close-jgit-resources

Conversation

@pditommaso
Copy link
Member

@pditommaso pditommaso commented Feb 3, 2026

Summary

  • Make AssetManager implement Closeable interface
  • Use try-with-resources for AssetManager in all CLI commands
  • Fix unclosed JGit resources in SCM repository strategies

Changes

CLI commands — use auto-closeable try to ensure AssetManager.close() is called:

  • CmdRun, CmdPull, CmdView, CmdClone, CmdInfo, CmdConfig, CmdDrop

SCM resource leaks — fix JGit objects that were never closed:

  • MultiRevisionRepositoryStrategy.revisionToCommitWithBareRepo(): was calling Git.open() on every invocation without closing — now uses cached getBareGit()
  • MultiRevisionRepositoryStrategy.getGit() / getBareGitWithLegacyFallback(): legacy fallback created uncached Git.open() on every call — added cached _legacyGit field
  • MultiRevisionRepositoryStrategy.createBareRepo(): Git.cloneRepository().call() returned a Git object that was discarded — now closed
  • MultiRevisionRepositoryStrategy.createSharedClone(): FileRepository was never closed — wrapped in try-with-resources
  • AssetManager.clone(): Git.cloneRepository().call() returned a Git object that was discarded — now closed
  • LegacyRepositoryStrategy.download(): same clone.call() leak — now closed
  • LocalRepositoryProvider.readBytes() / listDirectory(): RevWalk and TreeWalk were not properly closed on all code paths — wrapped in try-with-resources

Test plan

  • Run relevant unit tests (AssetManagerTest, MultiRevisionRepositoryStrategyTest, LocalRepositoryProviderTest) — all pass
  • Run make test to verify no regressions

AssetManager now implements Closeable and CLI commands wrap manager
usage in try-finally blocks to ensure JGit resources are properly
released. This prevents ~90 second shutdown delays caused by JGit
shutdown hooks cleaning up unclosed Repository objects.

Signed-off-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@netlify
Copy link

netlify bot commented Feb 3, 2026

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 4e74582
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/6989ac7a7a917d00091934c9

@pditommaso pditommaso marked this pull request as draft February 3, 2026 09:17
@bentsherman bentsherman requested a review from jorgee February 3, 2026 14:24
pditommaso and others added 3 commits February 7, 2026 16:30
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
- Use cached getBareGit() instead of Git.open() in revisionToCommitWithBareRepo()
- Cache legacy Git object to avoid repeated unclosed Git.open() calls
- Close Git objects returned by clone commands
- Wrap FileRepository in try-with-resources in createSharedClone()
- Use try-with-resources for RevWalk/TreeWalk in LocalRepositoryProvider

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso pditommaso marked this pull request as ready for review February 7, 2026 15:58
@jorgee
Copy link
Contributor

jorgee commented Feb 9, 2026

In the description I see a part about SCM resource leaks, but I don't see changes in this files. Is there a commit missing?

@pditommaso
Copy link
Member Author

Yep sorry. Just pushed 4e74582

Copy link
Contributor

@jorgee jorgee left a comment

Choose a reason for hiding this comment

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

Changes look fine

@pditommaso pditommaso merged commit 908e4c7 into master Feb 11, 2026
25 checks passed
@pditommaso pditommaso deleted the fix/close-jgit-resources branch February 11, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants