views:

1854

answers:

7

Despite closing streams in finally clauses I seem to constantly run into cleaning up problems when using Java. File.delete() fails to delete files, Windows Explorer fails too. Running System.gc() helps sometimes but nothing short of terminating the VM helps consistently and that is not an option.

Does anyone have any other ideas I could try? I use Java 1.6 on Windows XP.

UPDATE: FLAC code sample removed, the code worked if I isolated it.

UPDATE: More info, this happens in Apache Tomcat, Commons FileUpload is used to upload the file and could be the culprit, also I use Runtime.exec() to execute LAME in a separate process to encode the file, but that seems unlikely to cause this since ProcessExplorer clearly indicates that java.exe has a RW lock on the file and LAME terminates fine.

UPDATE: I am working with the assumption that there is a missing close() or a close() that does not get called somewhere in my code or external library. I just can't find it!

A: 

Make sure you have your close calls in the finally block not in the try block. If there is no try/finally because the method throws the exception then add a try/finally and put the close in there.

Look at the Windows Task Manager. For the Processes add the "Handles" column (under the View menu). Watch to see if the handles keep going up without ever dropping.

Use a profiler to see if you have Stream/Reader/Writer objects around that you do not think you should have.

EDIT:

Thanks for posting the code... off to see it. One thing - your close methods are not both guaranteed to execute - the first close might throw and then the second won't run.

EDIT 2:

final WavWriter wavWriter = new WavWriter(os); LACDecoder decoder = new FLACDecoder(is);

The above two lines will cause the strams to be kept in instance variables presumably. As a test see if you can set the stream references to null after the decoder.decode() call (make a decoder.cleanup() method perhaps). See if holding onto the closed streams is causing a problem.

Also, do you do any wrapping of the streams passed into the above constructors? If so you might have to close the streams via the wrappers.

TofuBeer
(In practice FileInputSteram.close will not actually throw, although that is no excuse.)
Tom Hawtin - tackline
Setting these variables to null shouldn't make any difference.
Eddie
it would make a difference if he is somehow keeping a hold of some of the other objects which in turn is causing the streams to be saved. THough once the stream is closed I cannot see why it would keep the file locked. I suspect he has some sort of resource leak.
TofuBeer
+3  A: 

I assume you're using jFlac. I downloaded jFlac 1.3 and tried your sample code on a flac freshly downloaded from the internet live music archive. For me, it worked. I even monitored it with ProcessExplorer and saw the file handles be opened and then released. Is your test code truly as simple as what you gave us, or is that a simplified version of your code? For me, once close() was called, the handle was released and the file was subsequently successfully deleted.

Try changing your infinite loop to:

File toDelete = new File(path);
if (!toDelete.delete()) {
  System.out.println("Could not delete " + path);
  System.out.println("Does it exist? " + toDelete.exists());
}

or if you want to keep looping, then put a 1 second sleep between attempts to delete the file. I tried this with JDK6 on WinXP Pro.

Don't forget to put a try/catch around your close() and log errors if the close throws an exception.

Eddie
Thanks Eddie, I will also make a simpler testcase and get back.
Jonas Klemming
You are right, that code was not the cause, the lock is created elsewhere in my app. Searching...
Jonas Klemming
A: 

Isn't that an empty while loop?

you have:

try
{
...code
}
finally
{
}
while (something);

put some whitespace in there, and you actually have:

try
{
...code
}
finally
{
}


while (something)
   ;

your while loop isn't related to your try/finally. if your original try statement fails and the file isn't created, that while loop will never complete, because the try/finally will never execute a second time.

did you intend to make that a do{ all your code } while (your while statement)? because that isn't what you have there.

EDIT to clarify: my suggestion would be to change your while loop to have more info of why it can't delete:

while (!file.delete())
{
    if (!file.exists())
        break; // the file doesn't even exist, of course delete will fail

    if (!file.canRead())
        break; // the file isn't readable, delete will fail

    if (!file.canWrite())
        break; // the file isn't writable, delete will fail
}

because if delete fails once, its just going to fail over and over and over, of course its going to hang there. you aren't changing the state of the file in the loop.

Now that you've added other info, like Tomcat, etc, is this a permissions issue? are you trying to write to a file that the user tomcat is running as (nobody?) vm can't create? or delete a file that the tomcat process can't delete?

If process explorer/etc say java has a lock on the file, then something still has an open stream using it. someone might have not properly called close() on whatever streams are writing to the file?

John Gardner
The while loop isn't empty, exactly. It says, while the file isn't successfully being deleted, try again to delete it.
Eddie
Yes, your while isn't "empty", but your while is "dumb". its checking the exact same state over and over, and the inside of the while loop doesn't do anything to change it. how many times do you want the OS to tell you "you can't delete that file" without putting in some code to figure out why?
John Gardner
A: 

Your code sample should definitely work. In fact I ran your it on Java 1.6/Vista with jflac 1.3 and the source file is deleted, without any looping.

I'm guessing in your case another process is keeping the file open, perhaps a desktop search indexer or an antivirus. You can procexp to find which process is actually holding onto the file.

Dan Berindei
I have now used ProcessExplorer and it clearly indicates java.exe to have a handle with Share flag RW.
Jonas Klemming
Since ProcessExplorer shows the file is opened for writing I would say it's not closed properly by commons-fileupload. Perhaps you can post the code that uses commons-fileupload?
Dan Berindei
Yes, but I just can't find that missing finally block? FileUpload seems to do things right...
Jonas Klemming
+2  A: 

The code you posted looks good - it should not cause the issues you are describing. I understand you posted just a piece of the code you have - can you try extracting just this part to a separate program, run it and see if the issue still happens? My guess is that there is some other place in the code that does new FileInputStream(path); and does not close the stream properly. You might be just seeing the results here when you try to delete the file.

Bogdan
I have now used ProcessExplorer and it clearly indicates java.exe to have a handle with Share flag RW. Does that mean I should be looking for unclosed FileOutputStreams?
Jonas Klemming
It is hard to guess what it is that you are doing with those files :-) But my guess is it should not be too hard to go through the code and find all the places where you potentially read/write that file and make sure the streams are closed.
Bogdan
A: 

If you are out of clues and ideas: In cygwin, cd to your javaroot and run something like:

find . -name '*.java' -print0 | xargs -0 grep "new.*new.*putStream"

It might provide a few suspects...

fredarin
A: 

Another thing to try since you're using Tomcat-- in your Context Descriptor (typically Tomcat/conf/Catalina/localhost/your-context.xml), you can set antiResourceLocking=true, which is designed to "avoid resource locking on Windows". The default for this (if you don't specify) is false. Worth a try.

Seth