Skip to content

Commit 908e4c7

Browse files
pditommasoclaude
pditommaso
and
claude
authored
Fix JGit resources not being closed in CLI commands (#6795)
* Fix JGit resources not being closed in CLI commands 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> * Improve res handling [ci fast] Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> * Fix unclosed JGit resources in SCM strategies - 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> --------- Signed-off-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3f6c6b3 commit 908e4c7

File tree

11 files changed

+224
-214
lines changed

11 files changed

+224
-214
lines changed

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdClone.groovy‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,25 +52,25 @@ class CmdClone extends CmdBase implements HubOptions {
5252
Plugins.init()
5353
// the pipeline name
5454
String pipeline = args[0]
55-
final manager = new AssetManager(pipeline, this)
55+
try (final manager = new AssetManager(pipeline, this)) {
56+
// the target directory is the second parameter
57+
// otherwise default the current pipeline name
58+
def target = new File(args.size()> 1 ? args[1] : manager.getBaseName())
59+
if( target.exists() ) {
60+
if( target.isFile() )
61+
throw new AbortOperationException("A file with the same name already exists: $target")
62+
if( !target.empty() )
63+
throw new AbortOperationException("Clone target directory must be empty: $target")
64+
}
65+
else if( !target.mkdirs() ) {
66+
throw new AbortOperationException("Cannot create clone target directory: $target")
67+
}
5668

57-
// the target directory is the second parameter
58-
// otherwise default the current pipeline name
59-
def target = new File(args.size()> 1 ? args[1] : manager.getBaseName())
60-
if( target.exists() ) {
61-
if( target.isFile() )
62-
throw new AbortOperationException("A file with the same name already exists: $target")
63-
if( !target.empty() )
64-
throw new AbortOperationException("Clone target directory must be empty: $target")
69+
manager.checkValidRemoteRepo()
70+
print "Cloning ${manager.getProjectWithRevision()} ..."
71+
manager.clone(target, revision, deep)
72+
print "\r"
73+
println "${manager.getProjectWithRevision()} cloned to: $target"
6574
}
66-
else if( !target.mkdirs() ) {
67-
throw new AbortOperationException("Cannot create clone target directory: $target")
68-
}
69-
70-
manager.checkValidRemoteRepo()
71-
print "Cloning ${manager.getProjectWithRevision()} ..."
72-
manager.clone(target, revision, deep)
73-
print "\r"
74-
println "${manager.getProjectWithRevision()} cloned to: $target"
7575
}
7676
}

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdConfig.groovy‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,13 +233,13 @@ class CmdConfig extends CmdBase {
233233
return file.parent ?: Paths.get('/')
234234
}
235235

236-
final manager = new AssetManager(path, revision)
237-
if( revision && manager.isUsingLegacyStrategy() ){
238-
log.warn("The local asset for ${path} does not support multi-revision - 'revision' option is ignored\n" +
239-
"Consider updating the project using 'nextflow pull ${path} -r $revision -migrate'")
236+
try (final manager = new AssetManager(path, revision)) {
237+
if( revision && manager.isUsingLegacyStrategy() ){
238+
log.warn("The local asset for ${path} does not support multi-revision - 'revision' option is ignored\n" +
239+
"Consider updating the project using 'nextflow pull ${path} -r $revision -migrate'")
240+
}
241+
return manager.isLocal() ? manager.localPath.toPath() : manager.configFile?.parent
240242
}
241-
manager.isLocal() ? manager.localPath.toPath() : manager.configFile?.parent
242-
243243
}
244244

245245
}

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdDrop.groovy‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ class CmdDrop extends CmdBase {
5050
@Override
5151
void run() {
5252
Plugins.init()
53-
new AssetManager(args[0]).drop(revision, force)
53+
try (final manager = new AssetManager(args[0])) {
54+
manager.drop(revision, force)
55+
}
5456
}
5557
}

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdInfo.groovy‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -75,25 +75,26 @@ class CmdInfo extends CmdBase {
7575
}
7676

7777
Plugins.init()
78-
final manager = new AssetManager(args[0])
79-
if( manager.isNotInitialized() ) {
80-
throw new AbortOperationException("Unknown project `${args[0]}`")
81-
}
78+
try (final manager = new AssetManager(args[0])) {
79+
if( manager.isNotInitialized() ) {
80+
throw new AbortOperationException("Unknown project `${args[0]}`")
81+
}
8282

83-
if( !format || format == 'text' ) {
84-
printText(manager,level)
85-
return
86-
}
83+
if( !format || format == 'text' ) {
84+
printText(manager,level)
85+
return
86+
}
8787

88-
def map = createMap(manager)
89-
if( format == 'json' ) {
90-
printJson(map)
91-
}
92-
else if( format == 'yaml' ) {
93-
printYaml(map)
88+
def map = createMap(manager)
89+
if( format == 'json' ) {
90+
printJson(map)
91+
}
92+
else if( format == 'yaml' ) {
93+
printYaml(map)
94+
}
95+
else
96+
throw new AbortOperationException("Unknown output format: $format");
9497
}
95-
else
96-
throw new AbortOperationException("Unknown output format: $format");
9798

9899
}
99100

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdPull.groovy‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -83,39 +83,41 @@ class CmdPull extends CmdBase implements HubOptions {
8383

8484
for( String proj : list ) {
8585
if( all ) {
86-
def branches = new AssetManager(proj).getBranchesAndTags(false).pulled as List<String>
87-
branches.each { rev -> pullProjectRevision(proj, rev) }
86+
try (def mgr = new AssetManager(proj)) {
87+
def branches = mgr.getBranchesAndTags(false).pulled as List<String>
88+
branches.each { rev -> pullProjectRevision(proj, rev) }
89+
}
8890
} else {
8991
pullProjectRevision(proj, revision)
9092
}
9193
}
9294
}
9395

9496
private pullProjectRevision(String project, String revision) {
95-
final manager = new AssetManager(project, this)
96-
97-
if( manager.isUsingLegacyStrategy() ) {
98-
if( migrate ) {
99-
log.info "Migrating ${project} revision ${revision} to multi-revision strategy"
100-
manager.setStrategyType(AssetManager.RepositoryStrategyType.MULTI_REVISION)
101-
} else {
102-
log.warn "The local asset for ${project} does not support multi-revision - Pulling with legacy strategy\n" +
103-
"Consider updating the project ${project} using '-migrate' option"
97+
try (final manager = new AssetManager(project, this)) {
98+
if( manager.isUsingLegacyStrategy() ) {
99+
if( migrate ) {
100+
log.info "Migrating ${project} revision ${revision} to multi-revision strategy"
101+
manager.setStrategyType(AssetManager.RepositoryStrategyType.MULTI_REVISION)
102+
} else {
103+
log.warn "The local asset for ${project} does not support multi-revision - Pulling with legacy strategy\n" +
104+
"Consider updating the project ${project} using '-migrate' option"
105+
}
104106
}
105-
}
106107

107-
if( revision )
108-
manager.setRevision(revision)
108+
if( revision )
109+
manager.setRevision(revision)
109110

110-
log.info "Checking ${manager.getProjectWithRevision()} ..."
111+
log.info "Checking ${manager.getProjectWithRevision()} ..."
111112

112-
def result = manager.download(revision, deep)
113-
manager.updateModules()
113+
def result = manager.download(revision, deep)
114+
manager.updateModules()
114115

115-
def scriptFile = manager.getScriptFile()
116-
String message = !result ? " done" : " $result"
117-
message += " - revision: ${scriptFile.revisionInfo}"
118-
log.info message
116+
def scriptFile = manager.getScriptFile()
117+
String message = !result ? " done" : " $result"
118+
message += " - revision: ${scriptFile.revisionInfo}"
119+
log.info message
120+
}
119121
}
120122

121123
}

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy‎

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -599,41 +599,42 @@ class CmdRun extends CmdBase implements HubOptions {
599599
/*
600600
* try to look for a pipeline in the repository
601601
*/
602-
def manager = new AssetManager(pipelineName, this)
603-
if( revision )
604-
manager.setRevision(revision)
605-
def repo = manager.getProjectWithRevision()
606-
607-
boolean checkForUpdate = true
608-
if( !manager.isRunnable() || latest ) {
609-
if( offline )
610-
throw new AbortOperationException("Unknown project `$repo` -- NOTE: automatic download from remote repositories is disabled")
611-
log.info "Pulling $repo ..."
612-
def result = manager.download(revision,deep)
613-
if( result )
614-
log.info " $result"
615-
checkForUpdate = false
616-
}
617-
// Warn if using legacy
618-
if( manager.isUsingLegacyStrategy() ){
619-
log.warn1 "This Nextflow version supports a new Multi-revision strategy for managing the SCM repositories, " +
620-
"but '${repo}' is single-revision legacy strategy - Please consider to update the repository with the 'nextflow pull -migrate' command."
621-
}
622-
// post download operations
623-
try {
624-
manager.checkout(revision)
625-
manager.updateModules()
626-
final scriptFile = manager.getScriptFile(mainScript)
627-
if( checkForUpdate && !offline )
628-
manager.checkRemoteStatus(scriptFile.revisionInfo)
629-
// return the script file
630-
return scriptFile
631-
}
632-
catch( AbortOperationException e ) {
633-
throw e
634-
}
635-
catch( Exception e ) {
636-
throw new AbortOperationException("Unknown error accessing project `$repo` -- Repository may be corrupted: ${manager.localPath}", e)
602+
try (def manager = new AssetManager(pipelineName, this)) {
603+
if( revision )
604+
manager.setRevision(revision)
605+
def repo = manager.getProjectWithRevision()
606+
607+
boolean checkForUpdate = true
608+
if( !manager.isRunnable() || latest ) {
609+
if( offline )
610+
throw new AbortOperationException("Unknown project `$repo` -- NOTE: automatic download from remote repositories is disabled")
611+
log.info "Pulling $repo ..."
612+
def result = manager.download(revision,deep)
613+
if( result )
614+
log.info " $result"
615+
checkForUpdate = false
616+
}
617+
// Warn if using legacy
618+
if( manager.isUsingLegacyStrategy() ){
619+
log.warn1 "This Nextflow version supports a new Multi-revision strategy for managing the SCM repositories, " +
620+
"but '${repo}' is single-revision legacy strategy - Please consider to update the repository with the 'nextflow pull -migrate' command."
621+
}
622+
// post download operations
623+
try {
624+
manager.checkout(revision)
625+
manager.updateModules()
626+
final scriptFile = manager.getScriptFile(mainScript)
627+
if( checkForUpdate && !offline )
628+
manager.checkRemoteStatus(scriptFile.revisionInfo)
629+
// return the script file
630+
return scriptFile
631+
}
632+
catch( AbortOperationException e ) {
633+
throw e
634+
}
635+
catch( Exception e ) {
636+
throw new AbortOperationException("Unknown error accessing project `$repo` -- Repository may be corrupted: ${manager.localPath}", e)
637+
}
637638
}
638639

639640
}

‎modules/nextflow/src/main/groovy/nextflow/cli/CmdView.groovy‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,35 +54,35 @@ class CmdView extends CmdBase {
5454
@Override
5555
void run() {
5656
Plugins.init()
57-
final manager = new AssetManager(args[0], revision)
58-
if( !manager.isLocal() )
59-
throw new AbortOperationException("Unknown project `${manager.getProjectWithRevision()}`")
60-
if( revision && manager.isUsingLegacyStrategy()){
61-
log.warn("The local asset ${args[0]} does not support multi-revision - 'revision' option is ignored\n" +
62-
"Consider updating the asset using 'nextflow pull ${args[0]} -r $revision -migrate'")
63-
}
64-
if( all ) {
65-
if( !quiet )
66-
println "== content of path: ${manager.localPath}"
57+
try (final manager = new AssetManager(args[0], revision)) {
58+
if( !manager.isLocal() )
59+
throw new AbortOperationException("Unknown project `${manager.getProjectWithRevision()}`")
60+
if( revision && manager.isUsingLegacyStrategy()){
61+
log.warn("The local asset ${args[0]} does not support multi-revision - 'revision' option is ignored\n" +
62+
"Consider updating the asset using 'nextflow pull ${args[0]} -r $revision -migrate'")
63+
}
64+
if( all ) {
65+
if( !quiet )
66+
println "== content of path: ${manager.localPath}"
6767

68-
manager.localPath.eachFile { File it ->
69-
println it.name
68+
manager.localPath.eachFile { File it ->
69+
println it.name
70+
}
7071
}
71-
}
7272

73-
else {
74-
/*
75-
* prints the script main file
76-
*/
77-
final script = manager.getMainScriptFile()
78-
if( !script.exists() )
79-
throw new AbortOperationException("Missing script file: '${script}'")
73+
else {
74+
/*
75+
* prints the script main file
76+
*/
77+
final script = manager.getMainScriptFile()
78+
if( !script.exists() )
79+
throw new AbortOperationException("Missing script file: '${script}'")
8080

81-
if( !quiet )
82-
println "== content of file: $script"
81+
if( !quiet )
82+
println "== content of file: $script"
8383

84-
script.readLines().each { println it }
84+
script.readLines().each { println it }
85+
}
8586
}
86-
8787
}
8888
}

‎modules/nextflow/src/main/groovy/nextflow/scm/AssetManager.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import org.eclipse.jgit.merge.MergeStrategy
5858

5959
@Slf4j
6060
@CompileStatic
61-
class AssetManager {
61+
class AssetManager implements Closeable {
6262

6363
/**
6464
* The folder all pipelines scripts are installed
@@ -782,7 +782,7 @@ class AssetManager {
782782
clone.setBranch(revision)
783783
if( deep )
784784
clone.setDepth(deep)
785-
clone.call()
785+
clone.call().close()
786786
}
787787

788788
/**

‎modules/nextflow/src/main/groovy/nextflow/scm/LegacyRepositoryStrategy.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class LegacyRepositoryStrategy extends AbstractRepositoryStrategy {
8686
.setCloneSubmodules(manifest.recurseSubmodules)
8787
if( deep )
8888
clone.setDepth(deep)
89-
clone.call()
89+
clone.call().close()
9090

9191
// git cli would automatically create a 'refs/remotes/origin/HEAD' symbolic ref pointing at the remote's
9292
// default branch. jgit doesn't do this, but since it automatically checked out the default branch on clone

0 commit comments

Comments
 (0)