Bug 34893

Summary: unzip task input file remains open after exception
Product: Ant Reporter: gjfdh
Component: Core tasksAssignee: Ant Notifications List <notifications>
Severity: critical CC: notifications
Priority: P2    
Version: 1.6.3   
Target Milestone: 1.6.4   
Hardware: All   
OS: Windows Server 2003   

Description gjfdh 2005-05-12 15:21:36 UTC
The JVM is left dirty after the unzip task throws an exception due to a 
corrupt or otherwise improperly formatted zip file.  The JVM is left holding 
the input file open.

Here's a test case.  The build file, below, uses the subant task to capture, 
log, and otherwise ignore the exception from unzip.  The build file creates a 
zero-length zip file (xxx.zip).  The unzip task is run on the zip and an 
exception is thrown.  The build then attempts to delete the zip and cannot do 
so because (I presume) the JVM is holding an open lock.  The only way to 
recover, once you are in this state, that I have found is to exit the JVM.

I looked through the ant source code but was unable to find the bug myself, so 
I don't have a suggested fix for this one.  Most users who run ant from the 
command line would not see this bug because the JVM would exit after the unzip 
exception.  However, this bug is important to users whose JVM continues to 
execute after ant is invoked (e.g. ant users who are invoking ant from within 
a greater Java or Jython program).  The open corrupt zip file can cause a 
logjam of errors when it cannot be removed.

<project default="default">
   <target name="do-unzip">
      <touch file="xxx.zip"/>
      <unzip src="xxx.zip" dest="."/>
   <target name="default">
      <subant target="do-unzip" failonerror="false">
        <fileset dir="." includes="build.xml"/>
      <delete file="xxx.zip"/>

C:\>%ant_home%\bin\ant -verbose
Apache Ant version 1.6.2 compiled on July 16 2004
Buildfile: build.xml
Detected Java version: 1.4 in: C:\Program Files\IBM\Java142\jre
Detected OS: Windows 2003
parsing buildfile C:\build.xml with URI = file:///C:/build.xml
Project base dir set to: C:\
Build sequence for target `default' is [default]
Complete build sequence is [default, do-unzip, ]

   [subant] calling target do-unzip in build file C:\build.xml
parsing buildfile C:\build.xml with URI = file:///C:/build.xml
Project base dir set to: C:\
   [subant] Entering C:\build.xml...
Build sequence for target `do-unzip' is [do-unzip]
Complete build sequence is [do-unzip, default, ]

    [unzip] Expanding: C:\xxx.zip into C:\
   [subant] Exiting C:\build.xml.
   [subant] Failure for target 'do-unzip' of: C:\build.xml
   [subant] The following error occurred while executing this line:
   [subant] C:\build.xml:4: Error while expanding C:\xxx.zip
   [delete] Deleting: C:\xxx.zip

C:\build.xml:10: Unable to delete file C:\xxx.zip
        at org.apache.tools.ant.taskdefs.Delete.execute(Delete.java:471)
        at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:275)
        at org.apache.tools.ant.Task.perform(Task.java:364)
        at org.apache.tools.ant.Target.execute(Target.java:341)
        at org.apache.tools.ant.Target.performTasks(Target.java:369)
        at org.apache.tools.ant.Project.executeTarget(Project.java:1214)
        at org.apache.tools.ant.Project.executeTargets(Project.java:1062)
        at org.apache.tools.ant.Main.runBuild(Main.java:673)
        at org.apache.tools.ant.Main.startAnt(Main.java:188)
        at org.apache.tools.ant.launch.Launcher.run(Launcher.java:196)
        at org.apache.tools.ant.launch.Launcher.main(Launcher.java:55)

Total time: 1 second
Comment 1 Stefan Bodewig 2005-05-12 22:02:28 UTC
I don't see how unzip would not close the file since it is inside a finally block.
Except for File.close() failing, but then we'd be out of luck anyway.

Could it be a timing issue?  One where Java has close the stream but your OS
hasn't recognized this?  Does it help if you put a short <sleep> in front of
the delete?
Comment 2 gjfdh 2005-05-12 23:41:28 UTC
Sleep doesn't help.  Some automated code I have has looped on this bug, 
repeatedly trying to delete the file, over an entire weekend with no luck.  
And, if it was a timing issue, in OS code, this same bug would have shown when 
deleting properly formatted zip files.  Assuming everything else is the same, 
the OS wouldn't care if the zip was formatted properly or not -- a close is a 

The test build.xml file I gave above works under unix systems

    [touch] Creating /tmp/xxx.zip
    [unzip] Expanding: /tmp/xxx.zip into /tmp
   [subant] Exiting /tmp/build.xml.
   [subant] Failure for target 'do-unzip' of: /tmp/build.xml
   [subant] The following error occurred while executing this line:
   [subant] /tmp/build.xml:4: Error while expanding /tmp/xxx.zip
   [delete] Deleting: /tmp/xxx.zip

by the delete semantics are different there.  You are allowed to delete open 
files hence this test doesn't show if the bug exists or not on unix -- the 
symptom is gone but the bug may or may not be there.

It is tricky.  I'm hoping that someone in the bugzilla audience may know 
something about zipfile or Windows or ... which can explain what's going on.
Comment 3 gjfdh 2005-05-13 00:26:33 UTC
I think it's a matter of finalizers.  

Expand.java creates a ZipFile.  An IOException is thrown during the ZipFile 
constructor when archive.seek() is called with a negative offset in the 
positionAtCentralDirectory method.  The instance variable, archive, is holding 
a RandomAccessFile object and eventually may be garbage collected.  But until 
then, the file remains open.  The zf.close() in Expand.java's finally is 
irrelevant here because the exception happened during the "zf = new 
ZipFile..." statement.

The fix is to change the ZipFile so it explicitly closes "archive".

New constructor:
    public ZipFile(File f, String encoding) throws IOException {
        this.encoding = encoding;
        archive = new RandomAccessFile(f, "r");
        try {
        } catch (IOException e) {
            throw e;

Old constructor (for comparision purposes):

    public ZipFile(File f, String encoding) throws IOException {
        this.encoding = encoding;
        archive = new RandomAccessFile(f, "r");

Here's some standalone test code which also exhibits the problem:

import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;

public class Test {
    public static void main( String[] args ) {
        final String fileName = "xxx.zip";
        //WORKS RandomAccessFile f2 = null;
    	try {
            // Touch the file
            new RandomAccessFile(fileName, "rw").close();

            // Perform a seek.  This throws an IOException    	    
            RandomAccessFile f2 = null;
    	    f2 = new RandomAccessFile(fileName, "r");
        } catch (IOException e) {
            /* WORKS
            try {
                System.out.println("Explictly closing the file");
            } catch (IOException e2) {
               WORKS */

         final boolean rc = new File(fileName).delete();

Leave the comments in the code, and the delete() method returns false.  
Uncomment the WORKS lines (and move the var declaration up) and the delete() 
method returns true.
Comment 4 Stefan Bodewig 2005-05-13 09:10:29 UTC
I could have been staring at the code for ages without realizing that the
constructor was throwing the exception.  Thanks!

A stack-trace would have been helpful.
Comment 5 Stefan Bodewig 2005-05-13 10:09:35 UTC
The same problem could happen in <untar> as well, <gunzip> and <bunzip2> seem
to be save.

Fixed in CVS, should be fixed in 1.6.4.  If you can build Ant from sources or
pick up a nightly build of 2005-05-14 or later to confirm it has been fixed,
that would be great.