3

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?

Miguel-F
  • 13,450
  • 6
  • 38
  • 63
Ennio
  • 1,147
  • 2
  • 17
  • 34
  • Does moving the close and delete into a `finally` block after the try/catch help? – Seanvm Dec 06 '18 at 06:40
  • 1
    What's the error message trace? Is the code contained in a .cfm or .cfc? 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. – SOS Dec 06 '18 at 20:14
  • Also the smtp server could be a possible source locking the file – jontro Dec 06 '18 at 22:14
  • Nothing to do with the error, but don't use `System.getProperties()`. That modifies settings across the entire JVM. Instead, just create a new [Properties](https://docs.oracle.com/javase/tutorial/essential/environment/properties.html) object. – SOS Dec 09 '18 at 00:37

1 Answers1

2

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.

Alex
  • 7,743
  • 1
  • 18
  • 38
  • This. Exactly this. As @Alex pointed out, extra vigilance is required with tasks involving f/s, database/network access, etc.. to ensure proper clean up of all resources. Also, regarding `this` scope: IF the parent cfc is being stored in a shared scope, then multiple threads could be trying to access the same `fileSource` - *at the same time* - which could cause the error mentioned. Since such race conditions typically occur only under load, it would also explain the sporadic nature of the error. – SOS Dec 09 '18 at 00:32