harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.com>
Subject Re: [classlib][luni] creating temporary files
Date Thu, 08 Jul 2010 03:25:48 GMT
On 2010-07-07 17:10, Tim Ellison wrote:
> On 06/Jul/2010 15:49, Jesse Wilson wrote:
>> On Sun, Jul 4, 2010 at 8:05 PM,<regisxu@apache.org>  wrote:
>>
>>> File.counter could be accessed by multiple threads, so use
>>> AtomicInteger to make sure each thread using different int
>>> value to create temp file.
>>
>> Is this a sufficient fix for the problem? The file system may be shared by
>> multiple concurrently-executing VMs. Coordinating access to temp files
>> within a single VM doesn't prevent other processes from grabbing the same
>> file names.
>>
>> A more robust solution might involve including the VM's process ID in the
>> filename. Or perhaps some kind of create-and-test protocol in a loop.
>
> I'm confused ... the code I'm looking at in File (before Regis' changes)
> do have the create in a loop.
>
> Not showing some checking etc, it says,
>
>    public static File createTempFile(String prefix, String suffix,
>            File directory) throws IOException {
>
>        File result;
>        do {
>            result = genTempFile(prefix, newSuffix, tmpDirFile);
>        } while (!result.createNewFile());
>        return result;
>    }
>

I never noticed there is loop here!! However, generating more 'unique' file name 
can help to reduce failed times of createNewFile().

>
> the genTempFile is having a good guess at creating a unique name, in
> pseudo code:
>
>    private static File genTempFile(String prefix, String suffix,
>        File directory) {
>
>        if atomic counter is not set, initialize to a random number
>        return File (dir, prefix + counter.getAndIncrement() + suffix);
>      }
>
>
> and the "while (!result.createNewFile())" loops while the creation
> reports the file exists.
>
> So what was the original problem that needed fixing?  Multiple threads
> should have a different value for the 'counter' and multiple VMs are
> either separated by the random number initialization, or have to spin on
> a test and re-gen a name if they collide.

The problems here I think is, we grow the identify number one by one, if the 
value is collide with two different VMs once, it will collide frequently. loop 
testing would be costly if createNewFile failed often.

>
>
>> On Tue, Jul 6, 2010 at 6:56 AM, Mark Hindess<mark.hindess@googlemail.com>wrote:
>>
>>> This breaks the build for the IBM VME (from developerWorks).  Since they
>>> don't have a sun.misc.Unsafe, so the AtomicInteger can't be resolved.
>>
>> If VME is to stay modern, they're going to need to support
>> java.util.concurrent. I don't think Harmony should provide a
>> lowest-common-denominator class library; supporting systems that don't have
>> j.u.c is an unnecessary handicap.
>
> Agreed.  As shown above this code already uses concurrent utilities in
> the existing algorithm anyways.
>
>> Particularly since they can implement AtomicInteger using less efficient
>> concurrency primitives. There's even a full
>> backport<http://backport-jsr166.sourceforge.net/>that could close some
>> gaps if the standard java.util.concurrent isn't
>> feasible for their VM.
>
> We should just assume that we can use concurrent.  VMs will have to
> support it as they see fit.
>
> Regards,
> Tim
>
>

This is test case for original issue:

public class TempFileTest {

     public static void main(String[] args) throws IOException {
         for (int repeat = 1; repeat <= 10; repeat++) {

             int limit = 100;
             final int thisRepeat = repeat;

             for (int i = 0; i < limit; i++) {
                 java.lang.Thread nextThread = new java.lang.Thread() {

                     public void run() {
                         try {
                             java.io.File newFile = java.io.File.createTempFile(
                                     "test", "", null);
                             newFile.delete();
                             newFile.mkdirs();
                         } catch (java.lang.Exception e) {
                             System.err.println("Repeat " + thisRepeat + " :: "
                                     + this.getName() + " :: !!Exception!!");
                             e.printStackTrace();
                         }
                         System.out.println("Repeat " + thisRepeat + " done");
                     }
                 };

                 nextThread.setName("Thread" + i);
                 nextThread.start();
             }
         }
     }
}

sometimes it threw exception without r960424:

java.io.IOException: Cannot create: /home/bahamut/sandbox/temp/test57313
	at java.io.File.createNewFile(File.java:1233)
	at java.io.File.createTempFile(File.java:1298)
	at TempFileTest$1.run(TempFileTest.java:20)

check file system, "/home/bahamut/sandbox/temp/test57313" is already existed, 
and it's a directory. If I changed "newFile.mkdirs()" to 
"newFile.createNewFile()", no exception thrown again. It seems another bug is 
exposed:

         File file = new File("help");
         System.out.println(file.mkdir());
         System.out.println(file.createNewFile());

RI output:
true
false

Harmony output:
true
Exception in thread "main" java.io.IOException: Cannot create: help
	at java.io.File.createNewFile(File.java:1233)
	at TempFileTest.main(TempFileTest.java:9)


-- 
Best Regards,
Regis.

Mime
View raw message