Another way to make temp-file names unique



  • <font size="3">private File createUniqueFile() {</font>
    <font size="3"> File tmp = null;</font>
    <font size="3"> </font>
    <font size="3"> while(tmp == null || tmp.exists() ) {</font>
    <font size="3"> try {</font>
    <font size="3"> Thread.sleep(1);</font>
    <font size="3"> } catch (InterruptedException e) {}</font>
    <font size="3"> tmp = new File(temp_path, System.currentTimeMillis() + ".tmp");</font>
    <font size="3"> }</font>
    <font size="3"> </font>
    <font size="3"> return tmp;</font>



  • Does that return a number between 0 and 1000, or a number between 0 and 2^63 - 1? If it's the latter then that's not a bad way to assign unique names to temp files, as long as the system doesn't require any form of concurrency.



  • This is one of the standard wrong ways to do it - it's a classical tempfile race condition. There is no guarantee that the returned filename does not exist. If temp_path is writeable by other users, it's a security hole.



  • between 0 and 2^63-1



  • Why even bother with the Thread.sleep(1)?  And then swallowing InterruptedException? This whole thing seems iffy...



  • People people people.... we all know the correct way to create a temp file:

       public class FileNotFoundException extends Exception {
    public FileNotFoundException(String fileName) {
    super(fileName);
    }
    }

    // Since wood was a living organism, and no two are alike, all wooden tables
    // should be subtly different, and so no two checksums should ever be identical!

    public String createTempFileName() {
    String theFileName = null;
    try {
    myCreateTempFileName();
    } catch (FileNotFoundException ffe) {
    theFileName = ffe.getMessage();
    }
    return theFileName;
    }

    private void myCreateTempFileName() throws FileFoundException, FileNotFoundException {
    Image snapshotOfYourPrintedDocOnWoodenTable = new Image(...);
    long sum = checksum(snapshotOfYourPrintedDocOnWoodenTable);
    String fileName = "wtf." + sum + "" + System.currentTimeMillis() + ".tmp";
    File file = new File(fileName);

     if (!file.exists()) {
        throw new FileNotFoundException(fileName);
     } else {
        myCreateTempFileName(); // recursive call takes time...
     }
    

    }



  • Funny... whats the point in the while loop if the file has a problem it will bail out due to an exception (because the File is created outside the try block) Something better might be like this

    object fileNameLocker = new object;
    File tmpFile = null;
    if(Directory.Exists(filePath))
    {
       lock(fileNameLocker)
       {
          do
          {
             string fileName = "/* Time in milliseconds */.tmp"
             Sleep(10);
          }
          while(File.Exists(filepath + fileName))

          tmpFile = new File(filePath, fileName);
       }
    }



  • sorry for leaving my try block out of the above code... I was in a rush and was trying to type up the html so it would look formatted right



  • @shadowman said:

    Why even bother with the Thread.sleep(1)?  And then swallowing InterruptedException? This whole thing seems iffy...

    InterruptedException is a checked exception in Java thrown from Thread.sleep().  You have to catch it, or put it in the 'throws' clause.  Not unreasonable at all - do you need to do anything if your sleep is interrupted?  Unless this is multithreaded code and you need to worry about threads waking each other up, just swallow the exception.

    As for why to bother with a sleep at all, I have found that the resolution of the System.currentTimeInMillis() method varies by platform.  We have some old test code that uses a call to currentTimeInMillis to generate field values for database columns with unique indices.  Unfortunately, on the faster machines this breaks because subsequent calls happen fast enough to end up with the same value.  The fix?  Add Thread.sleep(10) calls in several hundred strategic locations.  These have since been find+replaced over the years to become Thread.sleep(1000).  Needless to say, the test suite is now so unbearable slow that nobody runs it.





  • @livid said:

    I have found that the resolution of the System.currentTimeInMillis() method varies by platform.

    Which is a polite way of saying that Windows, unlike pretty much everything else, does not have millisecond timer precision.



  • @livid said:

    @shadowman said:

    Why even bother with the Thread.sleep(1)?  And then swallowing InterruptedException? This whole thing seems iffy...

    InterruptedException is a checked exception in Java thrown from Thread.sleep().  You have to catch it, or put it in the 'throws' clause.  Not unreasonable at all - do you need to do anything if your sleep is interrupted?  Unless this is multithreaded code and you need to worry about threads waking each other up, just swallow the exception.

    As for why to bother with a sleep at all, I have found that the resolution of the System.currentTimeInMillis() method varies by platform.  We have some old test code that uses a call to currentTimeInMillis to generate field values for database columns with unique indices.  Unfortunately, on the faster machines this breaks because subsequent calls happen fast enough to end up with the same value.  The fix?  Add Thread.sleep(10) calls in several hundred strategic locations.  These have since been find+replaced over the years to become Thread.sleep(1000).  Needless to say, the test suite is now so unbearable slow that nobody runs it.

    Nvm... need to learn to read... can you EVER delete your own posts?



  • @asuffield said:

    @livid said:

    I have found that the resolution of the System.currentTimeInMillis() method varies by platform.

    Which is a polite way of saying that Windows, unlike pretty much everything else, does not have millisecond timer precision.


    No, it's a polite way of saying that Windows, like pretty much eveything else, has millisecond timer precision -- but the JRE ignores the millisecond-precise timer routines in favor of a less-precise one.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.