Embedded GlassFish doesn't remove temporary directories if dispose() not called

[dmatej]

GlassFish Version (and build number)

7.1.0-SNAPSHOT

Problem Description

Following directories should be removed using shutdown hooks, including all content.

• gfembed
• fileinstall

They must not be also publicly visible - just for the current user.

Steps to reproduce

Run tests with embedded GlassFish (or just this test: EmbeddedVirtualServerHostNameTest - it stores the runtime in a local )

Or run Embedded GlassFish from a Java program as in the above test: glassfish.start() followed by glassfish.stop(), without calling glassfish.dispose()

[OndroMih]

Analysis for gfembed:

• a temporary directory with name starting with gfembed is created in https://github.com/eclipse-ee4j/glassfish/blob/19a22e91bc67fd935438c901482e2b542811d8ed/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/EmbeddedGlassFishRuntime.java#L174
  • a temp file given by the JVM is deleted and a directory with the same name is created
  • CHANGE NEEDED: this is the place where we should change permissions to make it visible only for the current user
• the directory is later deleted in glassfish.dispose() is called: https://github.com/eclipse-ee4j/glassfish/blob/19a22e91bc67fd935438c901482e2b542811d8ed/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/AutoDisposableGlassFish.java#L94
  • if glassfish.dispose() is not called, or if only glassfish.stop() is called without calling dispose() too, the directory is not deleted
  • CHANGE NEEDED: override the start() method in AutoDisposableGlassFish.java and create a JVM shutdown hook to either call dispose if it wasn't called (it throws an exception if it was called already), or just delete the temp directory if autoDelete && instanceRoot != null) && instanceRoot.exists()

[OndroMih]

I created another issue to fix the test and avoid leaving temporary directories after the test suite is executed: https://github.com/eclipse-ee4j/glassfish/issues/25607

[dmatej]

I created another issue to fix the test and avoid leaving temporary directories after the test suite is executed: #25607

Probably would be good if the test would be extended to test that the file is deleted; the shutdown hook is a good idea, while it should stay also in the dispose method, so the instance could be "recycled" in the future even without JVM restart.

[OndroMih]

Probably would be good if the test would be extended to test that the file is deleted; the shutdown hook is a good idea, while it should stay also in the dispose method, so the instance could be "recycled" in the future even without JVM restart.

I'm not sure how to extend the test to verify that the temp directory is deleted. The test doesn't know the directory name, only the gfembed prefix. It could verify that there are no directories starting with gfembed in the temp folder, but then it could fail also if there are such directories from some other execution or when a test is killed. Maybe it could remember all gfembed directories before running Embedded GlassFish and verify that the list is the same when the test is finished.

[avpinchuk]

Analysis for gfembed:

• a temporary directory with name starting with gfembed is created in

    [glassfish/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/EmbeddedGlassFishRuntime.java](https://github.com/eclipse-ee4j/glassfish/blob/19a22e91bc67fd935438c901482e2b542811d8ed/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/EmbeddedGlassFishRuntime.java#L174)
  
  
       Line 174
    in
    [19a22e9](/eclipse-ee4j/glassfish/commit/19a22e91bc67fd935438c901482e2b542811d8ed)
  
  
  
  
  
  
  
         if (!instanceRoot.delete() || !instanceRoot.mkdir()) { 
  

  • a temp file given by the JVM is deleted and a directory with the same name is created
  • CHANGE NEEDED: this is the place where we should change permissions to make it visible only for the current user

• the directory is later deleted in glassfish.dispose() is called:

    [glassfish/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/AutoDisposableGlassFish.java](https://github.com/eclipse-ee4j/glassfish/blob/19a22e91bc67fd935438c901482e2b542811d8ed/nucleus/core/bootstrap-osgi/src/main/java/org/glassfish/main/boot/embedded/AutoDisposableGlassFish.java#L94)
  
  
       Line 94
    in
    [19a22e9](/eclipse-ee4j/glassfish/commit/19a22e91bc67fd935438c901482e2b542811d8ed)
  
  
  
  
  
  
  
         public void dispose() throws GlassFishException { 
  

  • if glassfish.dispose() is not called, or if only glassfish.stop() is called without calling dispose() too, the directory is not deleted
  • CHANGE NEEDED: override the start() method in AutoDisposableGlassFish.java and create a JVM shutdown hook to either call dispose if it wasn't called (it throws an exception if it was called already), or just delete the temp directory if autoDelete && instanceRoot != null) && instanceRoot.exists()

fileinstall directory created by the Felix framework

[OndroMih]

fileinstall directory created by the Felix framework

Felix is not used in Embedded GlassFish and I didn't find any fileinstall directories after I ran the Embedded GlassFish test suite. So if there are any undeleted fileinstall directories, they must come from some other action.

[avpinchuk]

So if there are any undeleted fileinstall directories, they must come from some other action.

You're right. They come from full GlassFish.

https://github.com/apache/felix-dev/blob/0916f3d49c865851c2899e835d8614fc2f174f74/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java#L617

[OndroMih]

When GlassFish server (not Embedded) is started, it produces a fileinstall directory in temp area, e.g. /tmp/fileinstall-1120919830204976074/. So these undeleted files must be coming from a test suite for GlassFish server.

This directory is normally deleted on GlassFish server shutdown, even if it receives a TERM signal. It's not deleted only if GlassFish is killed by KILL signal (kill -9) or in other cases or forceful termination.

I think we can ignore the fileinstall directories as they are always deleted under usual conditions. @dmatej probably saw them only because of GlassFish was forcibly terminated before the Embedded GlassFish test suite was executed.

[dmatej]

No, I am right. GHA on Windows is bit more hysterical about it ... First in the issue I just noticed those files, but not the reason why they remain there. There's a lot of exceptions, so maybe this will need a serie of fixes ...

22:04:14.396527    INFO                 main                                                             . Shutdown procedure finished
22:04:14.422519  SEVERE                 main                                                             . Could not delete: C:\Users\RUNNER~1\AppData\Local\Temp\gfembed17348559710842114447tmp\lib\databases\ejbtimer\seg0\cf0.dat
java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\gfembed17348559710842114447tmp\lib\databases\ejbtimer\seg0\cf0.dat: The process cannot access the file because it is being used by another process
	at java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:92)
	at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
	at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
	at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:273)
	at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
	at java.base/java.nio.file.Files.delete(Files.java:1153)
	at org.glassfish.main.boot.embedded.AutoDisposableGlassFish.delete(AutoDisposableGlassFish.java:126)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
	at java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:261)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.glassfish.main.boot.embedded.AutoDisposableGlassFish.deleteRecursive(AutoDisposableGlassFish.java:116)
	at org.glassfish.main.boot.embedded.AutoDisposableGlassFish.dispose(AutoDisposableGlassFish.java:107)
	at org.glassfish.tests.embedded.ejb.basic.test.EmbeddedTest.test(EmbeddedTest.java:71)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:786)
	at org.junit.platform.commons.support.ReflectionSupport.invokeMethod(ReflectionSupport.java:514)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)

...

Could not delete: C:\Users\RUNNER~1\AppData\Local\Temp\gfembed17348559710842114447tmp

[avpinchuk]

Running mvn clean install on Windows leaves a lot of garbage:

[dmatej]

Yeah, I am using Linux, so even there it can happen. Partially it is also because many tests still don't use @TempDir, but another part might be resource leaks or some "issues with causality".