tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 30362] - Tomcat eats up file handers
Date Wed, 28 Jul 2004 14:36:47 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=30362>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=30362

Tomcat eats up file handers





------- Additional Comments From sd@mediaclockwork.de  2004-07-28 14:36 -------
okay, first of all the tomcat is on a linux box - no windows !

So here are my concerns in WebappClassloader:
    public void closeJARs(boolean force) {
        if (jarFiles.length > 0) {
            try {
                synchronized (jarFiles) {
                    if (force || (System.currentTimeMillis()
                                  > (lastJarAccessed + 90000))) {
                        for (int i = 0; i < jarFiles.length; i++) {
                            if (jarFiles[i] != null) {
                                jarFiles[i].close();
                                jarFiles[i] = null;
                            }
                        }
                    }
                }
            } catch (IOException e) {
                log.warn("Failed to close JAR", e);
            }
        }
    }

If you get an IOException the for-loop will break, and not run through the rest 
of possible still open files. If you take then a look at 
    protected void openJARs() {
        if (started && (jarFiles.length > 0)) {
            lastJarAccessed = System.currentTimeMillis();
            if (jarFiles[0] == null) {
                try {
                    for (int i = 0; i < jarFiles.length; i++) {
                        jarFiles[i] = new JarFile(jarRealFiles[i]);
                    }
                } catch (IOException e) {
                    log.warn("Failed to open JAR", e);
                }
            }
        }
    }
Only the first array entry is checked for "null", so if there are other not 
closed entries, they will simply overwritten without closing them first. I'm 
not sure, whether this is really the point of failure, but this was my first 
guess looking at the code. So a solution would be e.g.:

    protected void openJARs() {
        if (started && (jarFiles.length > 0)) {
            lastJarAccessed = System.currentTimeMillis();
            if (jarFiles[0] == null) {
                try {
                    for (int i = 0; i < jarFiles.length; i++) {
                        if (jarFiles[i]!=null){// would prefer a isClosed() 
check too, but seems not be available
                            jarFiles[i].close(); 
                            jarFiles[i]=null;
                        }
                        jarFiles[i] = new JarFile(jarRealFiles[i]);
                    }
                } catch (IOException e) {
                    log.warn("Failed to open JAR", e);
                }
            }
        }
    }

or in closeJARs just move the try-catch inside the for-loop (it's expensive, I 
know), but may be a simple
finally{
 continue;
}
would do too, because the IOException will break the loop and the finally will 
continue it, I'm not sure whether this works with an inner synchronized block - 
never thought about this, I gues from JVM spec it should work ?

Still not sure, that this is really the point, but I can assure you, I got tons 
of this exception.

---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org


Mime
View raw message