The files opened by OnDiskSemanticSpace aren't closed until the JVM exits. On Windows this leads to locks on the files so they can't be removed while the java process is alive. The SemanticSpace-Interface should be extended with a close- or destroy-method in which OnDiskSemanticSpace does close the files.
Comment #1
Posted on Sep 20, 2011 by Quick HorseWhich files are you specifically referring to? In some cases, the backing sspace file has to be maintained throughout the lifetime of the process because we do random access on the file when the whole space can't fit into ram. Is there a particular reason you want to delete the backing sspace while the sspace is running?
Comment #2
Posted on Sep 21, 2011 by Massive KangarooActually you are pointing to the problem. Here we have a long time running process (ApplicationServer) which creates multiple large temporary sspace (~1GB) during runtime. The sspace doesn't fit into ram so we use SPARSE_BINARY format and OnDiskSemanticSpace. In loadOffsetsFromFormat() the RandomAccessFile is opened, stored in binarySSpace and never closed. Because a native file handle is attached to RandomAccessFile the sspace-file is locked on Windows and can't be deleted. So we have a lot of large dead temp files lying around which can't be deleted for a long time.
Comment #3
Posted on Sep 21, 2011 by Happy RhinoIs it that the files are never unlocked when the program finishes, and therefore the file can't be deleted until Windows cleans up the file locks?
Is it possible to tell us more about your use case? Typically, we've left the files in place assuming that the user might access them at any point. We could potentially close those files and then re-open as necessary, but that incurs some overhead.
Comment #4
Posted on Sep 22, 2011 by Massive KangarooThe files are unlocked when the program is finished. Then they can be removed manually. They can't be removed by the application without a restart.
Our use case is to create one huge semantic space from time to time which is used only once.
Closing and reopening the files as necessary would slow the usage down. I thought of a way to close or destroy a SemanticSpace, so resources of the SemanticSpace (eg. open files) can be released. In OnDiskSemanticSpace it would look like this:
@Override public void close() { if (binarySSpace != null) { binarySSpace.close(); } }
Comment #5
Posted on Sep 23, 2011 by Quick HorseI think this would be a pretty weird addition to the SemanticSpace interface, but we could definitely add the close method to specific models that need to close files. If your progam is just using OnDiskSemanticSpace in this manner, then you could forfit the use of the interface and just keep these object declared as OnDiskSemanticSpace. ALternatively, we could overload the finalize() method and code it carefully so that files are only ever closed once. This would let you always do
// Close out any files in the SemanticSpace sspace.finalize();
The only issue with finalize is that it greatly complicates the garbage collection process.
I find it kind of frustrating that java doesn't just automatically close the file handler when a File object gets garbage collected.
Comment #6
Posted on Sep 26, 2011 by Massive KangarooI wrote a small Test which shows that closing the RandomAccessFile in finalize() would work (see attachment). If you remove the "System.gc()" the file can't be deleted. I would prefer an additional close()-method in OnDiskSemanticSpace though, because then you don't need to wait for the garbage collection to be able to remove the file.
- Test.java 818
Comment #7
Posted on Sep 26, 2011 by Happy RhinoThe close method seems like the best call for cleaning up the resources when you're done with the OnDiskSemanticSpace. It would actually be nice to use the new AutoCloseable interface from JSE7 which would let you do something like
try (OnDiskSemanticSpace sspace = new OnDiskSemanticSpace(...) {
// do things there
}
and have it close the backing RandomAccessFile in the event of an exception, or when you're finished with the sspace itself.
I'm not sure if we're ready to move to JSE7, but certainly putting the close() method in seems like the best choice here.
Status: Accepted
Labels:
Type-Defect
Priority-Medium