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,<regi...@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.hind...@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.
