ColdFusion 11 not closing java.io.FileInputStream

Question

I have a script in ColdFusion that is reading some files .EML from a local SMTP server and extracting some data from the files.

I have everything working fine, but from time to time the files gets locked and I cannot delete the file and I get the following error in ColdFusion.

ColdFusion could not delete the file C:/inetpub/mailroot/Queue/NTFS_AAAAAAAAAAAAAAAAAA.EML for an unknown reason.

Here is the code I'm using

<cfscript>
props = createObject("java", "java.lang.System").getProperties();

props.put( javacast("string", "mail.host"), javacast("string", "localhost"));
props.put( javacast("string", "mail.transport.protocol"),  javacast("string", "smtp"));

mailSession = createObject("java", "javax.mail.Session").getDefaultInstance(props, javacast("null", ""));

fileList = directoryList('C:\inetpub\mailroot\Queue\');

for (x=1; x LTE ArrayLen(fileList); x=x+1) {
    pathToEmailFile = fileList[x];

    this.fileSource = createObject("java", "java.io.FileInputStream").init(pathToEmailFile);

    try {
        message = createObject("java", "javax.mail.internet.MimeMessage").init(mailSession, this.fileSource);

        bodyData = message.getContent();
        bodyPart = bodyData.getBodyPart(javacast("int", 0)).getContent();
        from = reMatchNoCase('[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,63}', message.getFrom()[1].toString())[1];
        subject = message.getSubject();

        // CALL FUNCTION TO PROCESS DATA HERE
    } catch (any e) {
        // CLOSE THE FILE IF THERE IS ANY ERROR
        this.fileSource.close();
        writeDump(e);
    }
    this.fileSource.close();
    fileDelete(pathToEmailFile);
}
</cfscript>

Am I forgetting to close anything else and that is why it is causing the file to be locked?

Answer 1

The constructor of FileInputStream attempts to open a file stream, so it should to be part of your try block. Reminder: Always prepare for failure if the operation's result is out of your hands (e.g. filesystem, database and networking). You also need to make sure that you try your best to close the file stream, regardless of what happened after opening it, to ensure that the file handle is freed.

That's what the finally block is for. This block is executed regardless of what happens in the try and catch block.

• No exception occured? finally is executed at the end.
• Exception occured, catch is executed? finally is executed at the end.
• Exception occured, catch doesn't cover the exception? finally is executed at the end.
• Exception occured, catch is executed, but throws another exception? finally is executed at the end.

However, when executing finally, you might not know the exact circumstances you are in regarding the file stream (e.g. the file stream couldn't even be opened to begin with, so you won't be able to close it either). Thus you should still just try to close the stream.

for (x=1; x LTE ArrayLen(fileList); x=x+1) {
    pathToEmailFile = fileList[x];

    deleteFile = false;

    try {
        this.fileSource = createObject("java", "java.io.FileInputStream").init(pathToEmailFile);
        // your extraction code...
        deleteFile = true; // extraction succeeded, delete file after releasing it
    } catch (any e) {
        // log the exception
    } finally {
        // try to close the file stream
        try {
            this.fileSource.close();
        } catch (any e) {
            // file stream was probably not even opened
        }
    }

    if (deleteFile) {
        try {
            fileDelete(pathToEmailFile);
        } catch (any e) {
            // file could not be deleted, probably for the same reasons it failed previously
        }
    }
}

I'm not sure if you really intend to delete the file regardless of the extraction result, so I added logic for that and you decide for yourself.

Also, Ageax correctly observed:

I noticed the code uses this. scope, which might create a problem if the container is a cfc stored in a shared scope i.e. race conditions.

Is there a reason for having fileSource in the this scope? You shouldn't use a public field (this can be access from outside of a component) for a temporary variable.