You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Tim Ellison <t....@gmail.com> on 2010/07/07 11:10:16 UTC
[classlib][luni] creating temporary files (was: Re: svn commit: r960424
- /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java)
On 06/Jul/2010 15:49, Jesse Wilson wrote:
> On Sun, Jul 4, 2010 at 8:05 PM, <re...@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;
}
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.
> On Tue, Jul 6, 2010 at 6:56 AM, Mark Hindess <ma...@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
Re: [classlib][luni] creating temporary files
Posted by Regis <xu...@gmail.com>.
On 2010-07-08 15:22, Mark Hindess wrote:
>
> In message<4C...@gmail.com>, Regis writes:
>>
>> 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)
>
> I suspect this is an IBM VME bug not a Harmony bug - specifically the
> j9 port library. Did you test with DRLVM? We don't want to workaround
> bugs in the j9 port library in classlib.
>
> Regards,
> Mark.
>
>
>
Yes, it's only happened on IBM VME.
--
Best Regards,
Regis.
Re: [classlib][luni] creating temporary files
Posted by Mark Hindess <ma...@googlemail.com>.
In message <4C...@gmail.com>, Regis writes:
>
> 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)
I suspect this is an IBM VME bug not a Harmony bug - specifically the
j9 port library. Did you test with DRLVM? We don't want to workaround
bugs in the j9 port library in classlib.
Regards,
Mark.
Re: [classlib][luni] creating temporary files
Posted by Regis <xu...@gmail.com>.
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,<re...@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<ma...@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.