You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Mark Hindess <ma...@googlemail.com> on 2010/07/06 15:56:08 UTC
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Regis,
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.
Any ideas how to fix this?
Also, the luni.jar manifest says:
java.util.concurrent;resolution:=optional,
I wonder when it becomes non-optional. Personally, I'd say breaking
java.io.File would be enough to make it mandatory.
Regards,
Mark.
In message <20...@eris.apache.org>, regisxu@apache.org
writes:
>
> Author: regisxu
> Date: Mon Jul 5 03:05:16 2010
> New Revision: 960424
>
> URL: http://svn.apache.org/viewvc?rev=960424&view=rev
> Log:
> make File.createTempFile thread-safe to avoid to return the same file multipl
> e times
>
> File.counter could be accessed by multiple threads, so use AtomicInteger to m
> ake
> sure each thread using different int value to create temp file.
>
> Modified:
> harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
> ile.java
>
> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/jav
> a/io/File.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/module
> s/luni/src/main/java/java/io/File.java?rev=960424&r1=960423&r2=960424&view=di
> ff
> =============================================================================
> =
> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
> ile.java (original)
> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
> ile.java Mon Jul 5 03:05:16 2010
> @@ -24,6 +24,7 @@ import java.security.AccessController;
> import java.security.SecureRandom;
> import java.util.ArrayList;
> import java.util.List;
> +import java.util.concurrent.atomic.AtomicInteger;
>
> import org.apache.harmony.luni.internal.io.FileCanonPathCache;
> import org.apache.harmony.luni.util.DeleteOnExit;
> @@ -79,7 +80,7 @@ public class File implements Serializabl
> public static final String pathSeparator;
>
> /* Temp file counter */
> - private static int counter;
> + private static AtomicInteger counter = new AtomicInteger(0);
>
> private static boolean caseSensitive;
>
> @@ -1300,13 +1301,13 @@ public class File implements Serializabl
> }
>
> private static File genTempFile(String prefix, String suffix, File direc
> tory) {
> - if (counter == 0) {
> + if (counter.get() == 0) {
> int newInt = new SecureRandom().nextInt();
> - counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
> + counter.compareAndSet(0, ((newInt / 65535) & 0xFFFF) + 0x2710);
> }
> StringBuilder newName = new StringBuilder();
> newName.append(prefix);
> - newName.append(counter++);
> + newName.append(counter.getAndIncrement());
> newName.append(suffix);
> return new File(directory, newName.toString());
> }
>
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Mark Hindess <ma...@googlemail.com>.
In message <4C...@gmail.com>, Tim Ellison writes:
>
> On 06/Jul/2010 14:56, Mark Hindess wrote:
> > Regis,
> >
> > 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.
> >
> > Any ideas how to fix this?
> >
> > Also, the luni.jar manifest says:
> >
> > java.util.concurrent;resolution:=optional,
> >
> > I wonder when it becomes non-optional. Personally, I'd say breaking
> > java.io.File would be enough to make it mandatory.
>
> :-)
>
> To save you wondering, the imports are 'optional' when they are only
> required by the tests for a module, so the new dependency in this commit
> should indeed change concurrent to be a hard requirement for luni.
> We use the same manifest file in the Eclipse project (containing the
> impl and test together) as we do in the impl JAR, which is why we have
> to accommodate the tests in the declarations.
Ok. That was what I suspected but then I wondered why the concurrent
dependency didn't have " hy_usage=test" like the other optional
dependencies.
Regards,
Mark.
> > In message <20...@eris.apache.org>, regisxu@apache.org
> > writes:
> >> Author: regisxu
> >> Date: Mon Jul 5 03:05:16 2010
> >> New Revision: 960424
> >>
> >> URL: http://svn.apache.org/viewvc?rev=960424&view=rev
> >> Log:
> >> make File.createTempFile thread-safe to avoid to return the same file mult
> ipl
> >> e times
> >>
> >> File.counter could be accessed by multiple threads, so use AtomicInteger t
> o m
> >> ake
> >> sure each thread using different int value to create temp file.
> >>
> >> Modified:
> >> harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/i
> o/F
> >> ile.java
> >>
> >> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/
> jav
> >> a/io/File.java
> >> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/mod
> ule
> >> s/luni/src/main/java/java/io/File.java?rev=960424&r1=960423&r2=960424&view
> =di
> >> ff
> >> ==========================================================================
> ===
> >> =
> >> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/i
> o/F
> >> ile.java (original)
> >> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/i
> o/F
> >> ile.java Mon Jul 5 03:05:16 2010
> >> @@ -24,6 +24,7 @@ import java.security.AccessController;
> >> import java.security.SecureRandom;
> >> import java.util.ArrayList;
> >> import java.util.List;
> >> +import java.util.concurrent.atomic.AtomicInteger;
> >>
> >> import org.apache.harmony.luni.internal.io.FileCanonPathCache;
> >> import org.apache.harmony.luni.util.DeleteOnExit;
> >> @@ -79,7 +80,7 @@ public class File implements Serializabl
> >> public static final String pathSeparator;
> >>
> >> /* Temp file counter */
> >> - private static int counter;
> >> + private static AtomicInteger counter = new AtomicInteger(0);
> >>
> >> private static boolean caseSensitive;
> >>
> >> @@ -1300,13 +1301,13 @@ public class File implements Serializabl
> >> }
> >>
> >> private static File genTempFile(String prefix, String suffix, File di
> rec
> >> tory) {
> >> - if (counter == 0) {
> >> + if (counter.get() == 0) {
> >> int newInt = new SecureRandom().nextInt();
> >> - counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
> >> + counter.compareAndSet(0, ((newInt / 65535) & 0xFFFF) + 0x2710
> );
> >> }
> >> StringBuilder newName = new StringBuilder();
> >> newName.append(prefix);
> >> - newName.append(counter++);
> >> + newName.append(counter.getAndIncrement());
> >> newName.append(suffix);
> >> return new File(directory, newName.toString());
> >> }
> >>
> >
> >
> >
>
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Tim Ellison <t....@gmail.com>.
On 06/Jul/2010 14:56, Mark Hindess wrote:
> Regis,
>
> 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.
>
> Any ideas how to fix this?
>
> Also, the luni.jar manifest says:
>
> java.util.concurrent;resolution:=optional,
>
> I wonder when it becomes non-optional. Personally, I'd say breaking
> java.io.File would be enough to make it mandatory.
:-)
To save you wondering, the imports are 'optional' when they are only
required by the tests for a module, so the new dependency in this commit
should indeed change concurrent to be a hard requirement for luni.
We use the same manifest file in the Eclipse project (containing the
impl and test together) as we do in the impl JAR, which is why we have
to accommodate the tests in the declarations.
Regards,
Tim
> In message <20...@eris.apache.org>, regisxu@apache.org
> writes:
>> Author: regisxu
>> Date: Mon Jul 5 03:05:16 2010
>> New Revision: 960424
>>
>> URL: http://svn.apache.org/viewvc?rev=960424&view=rev
>> Log:
>> make File.createTempFile thread-safe to avoid to return the same file multipl
>> e times
>>
>> File.counter could be accessed by multiple threads, so use AtomicInteger to m
>> ake
>> sure each thread using different int value to create temp file.
>>
>> Modified:
>> harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java
>>
>> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/jav
>> a/io/File.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/module
>> s/luni/src/main/java/java/io/File.java?rev=960424&r1=960423&r2=960424&view=di
>> ff
>> =============================================================================
>> =
>> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java (original)
>> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java Mon Jul 5 03:05:16 2010
>> @@ -24,6 +24,7 @@ import java.security.AccessController;
>> import java.security.SecureRandom;
>> import java.util.ArrayList;
>> import java.util.List;
>> +import java.util.concurrent.atomic.AtomicInteger;
>>
>> import org.apache.harmony.luni.internal.io.FileCanonPathCache;
>> import org.apache.harmony.luni.util.DeleteOnExit;
>> @@ -79,7 +80,7 @@ public class File implements Serializabl
>> public static final String pathSeparator;
>>
>> /* Temp file counter */
>> - private static int counter;
>> + private static AtomicInteger counter = new AtomicInteger(0);
>>
>> private static boolean caseSensitive;
>>
>> @@ -1300,13 +1301,13 @@ public class File implements Serializabl
>> }
>>
>> private static File genTempFile(String prefix, String suffix, File direc
>> tory) {
>> - if (counter == 0) {
>> + if (counter.get() == 0) {
>> int newInt = new SecureRandom().nextInt();
>> - counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
>> + counter.compareAndSet(0, ((newInt / 65535) & 0xFFFF) + 0x2710);
>> }
>> StringBuilder newName = new StringBuilder();
>> newName.append(prefix);
>> - newName.append(counter++);
>> + newName.append(counter.getAndIncrement());
>> newName.append(suffix);
>> return new File(directory, newName.toString());
>> }
>>
>
>
>
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Regis <xu...@gmail.com>.
On 2010-07-06 21:56, Mark Hindess wrote:
>
> Regis,
>
> 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.
>
> Any ideas how to fix this?
>
> Also, the luni.jar manifest says:
>
> java.util.concurrent;resolution:=optional,
>
> I wonder when it becomes non-optional. Personally, I'd say breaking
> java.io.File would be enough to make it mandatory.
>
> Regards,
> Mark.
>
> In message<20...@eris.apache.org>, regisxu@apache.org
> writes:
>>
>> Author: regisxu
>> Date: Mon Jul 5 03:05:16 2010
>> New Revision: 960424
>>
>> URL: http://svn.apache.org/viewvc?rev=960424&view=rev
>> Log:
>> make File.createTempFile thread-safe to avoid to return the same file multipl
>> e times
>>
>> File.counter could be accessed by multiple threads, so use AtomicInteger to m
>> ake
>> sure each thread using different int value to create temp file.
>>
>> Modified:
>> harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java
>>
>> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/jav
>> a/io/File.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/module
>> s/luni/src/main/java/java/io/File.java?rev=960424&r1=960423&r2=960424&view=di
>> ff
>> =============================================================================
>> =
>> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java (original)
>> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/F
>> ile.java Mon Jul 5 03:05:16 2010
>> @@ -24,6 +24,7 @@ import java.security.AccessController;
>> import java.security.SecureRandom;
>> import java.util.ArrayList;
>> import java.util.List;
>> +import java.util.concurrent.atomic.AtomicInteger;
>>
>> import org.apache.harmony.luni.internal.io.FileCanonPathCache;
>> import org.apache.harmony.luni.util.DeleteOnExit;
>> @@ -79,7 +80,7 @@ public class File implements Serializabl
>> public static final String pathSeparator;
>>
>> /* Temp file counter */
>> - private static int counter;
>> + private static AtomicInteger counter = new AtomicInteger(0);
>>
>> private static boolean caseSensitive;
>>
>> @@ -1300,13 +1301,13 @@ public class File implements Serializabl
>> }
>>
>> private static File genTempFile(String prefix, String suffix, File direc
>> tory) {
>> - if (counter == 0) {
>> + if (counter.get() == 0) {
>> int newInt = new SecureRandom().nextInt();
>> - counter = ((newInt / 65535)& 0xFFFF) + 0x2710;
>> + counter.compareAndSet(0, ((newInt / 65535)& 0xFFFF) + 0x2710);
>> }
>> StringBuilder newName = new StringBuilder();
>> newName.append(prefix);
>> - newName.append(counter++);
>> + newName.append(counter.getAndIncrement());
>> newName.append(suffix);
>> return new File(directory, newName.toString());
>> }
>>
>
>
>
Fixed at r961242, instead of using 'synchronized'. I noticed that
AbstractSelector.java uses AtomicBoolean, does IBM VME only not support
AtomicInteger or AtomicLong?
--
Best Regards,
Regis.
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Regis <xu...@gmail.com>.
On 2010-07-07 17:14, Tim Ellison wrote:
> On 07/Jul/2010 05:49, Regis wrote:
>> On 2010-07-06 22: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.
>>>
>>
>> At r961242, I used counter value generated first time to identify
>> different VM instances, I think that's enough to avoid name conflicts.
>
> Huh? adding more local counters into the string in this VM isn't going
> to help.
>
> How does this code identify the VM instance?
>
>
> Regards,
> Tim
>
>
>
I assume the following code can generate different values of counterBase for
different VMs:
int newInt = new SecureRandom().nextInt();
counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
counterBase = counter;
counterBase do have chance to get the same value cross VM instances, but given
there would not be many VM instances running concurrently, this way can avoid
all most of conflicts with little extra efforts.
--
Best Regards,
Regis.
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Tim Ellison <t....@gmail.com>.
On 07/Jul/2010 05:49, Regis wrote:
> On 2010-07-06 22: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.
>>
>
> At r961242, I used counter value generated first time to identify
> different VM instances, I think that's enough to avoid name conflicts.
Huh? adding more local counters into the string in this VM isn't going
to help.
How does this code identify the VM instance?
Regards,
Tim
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Regis <xu...@gmail.com>.
On 2010-07-06 22: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.
>
At r961242, I used counter value generated first time to identify different VM
instances, I think that's enough to avoid name conflicts.
>
> 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.
>
> 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.
>
--
Best Regards,
Regis.
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.
[classlib][luni] creating temporary files (was: Re: svn commit: r960424
- /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java)
Posted by Tim Ellison <t....@gmail.com>.
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: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Mark Hindess <ma...@googlemail.com>.
In message <AA...@mail.gmail.com>,
Jesse Wilson writes:
>
> On Sun, Jul 4, 2010 at 8:05 PM, <re...@apache.org> wrote:
>
> 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.
For what it is worth, I agree. I was not necessarily anticipating the
change being reverted. I would not be unhappy with our breaking the
obsolete VME. My main purpose in sending my mail was to make sure we
did it knowingly with good reason rather than inadvertently because we
weren't paying attention.
> 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.
Useful reference. Thanks.
Regards,
Mark.
Re: svn commit: r960424 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/io/File.java
Posted by Jesse Wilson <je...@google.com>.
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.
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.
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.