[
https://issues.apache.org/jira/browse/LUCENE-4639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13537346#comment-13537346
]
Dawid Weiss commented on LUCENE-4639:
-------------------------------------
{noformat}
+ if (counter == null) {
+ synchronized (counterLock) {
+ if (counter == null) {
+ final Random random = new
Random(RandomizedContext.current().getRandom().nextLong());
+ int value = (random.nextInt() / 65535) & 0xFFFF; // make sure the
number is small and positive
+ // need to update counterBase before counter (read-after volatility)
+ counterBase = Integer.toString(value);
+ counter = new AtomicInteger(value);
+ }
{noformat}
This is confusing with a volatile and a critical section. I'd just leave the
counter as a non-volatile, regular field (not even an atomic int) and always
enter the synchronized block for lazy updates. Optimizing this makes no sense
to me, it's a no-cost anyway. Also:
{code}
(random.nextInt() / 65535) & 0xFFFF
{code}
is probably just as good as doing the masking:
{code}
random.nextInt() & 0xFFFF
{code}
I'd say +1 for the patch, even if I agree with Robert that running with
multiple JVMs in a shared folder is probably asking for a 10 hour debugging
session hunting for a heisenbug... But the patch is an improvement because it
allows doing repeated tests that leave unremovable stuff behind from within the
_same_ JVM (which can be done with a repeat annotation or with a system
property).
> Improving _TestUtil.getTempDir
> ------------------------------
>
> Key: LUCENE-4639
> URL: https://issues.apache.org/jira/browse/LUCENE-4639
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Shai Erera
> Assignee: Shai Erera
> Priority: Minor
> Attachments: LUCENE-4639.patch, LUCENE-4639.patch, LUCENE-4639.patch
>
>
> Spinoff from here:
> http://lucene.472066.n3.nabble.com/TestUtil-getTempFile-may-fail-on-quot-Access-Denied-quot-td4028048.html.
> _TestUtil.getTempDir uses createTempFile and then deletes the file. While
> this usually works, if someone runs tests by multiple JVMs and does not
> ensure each JVM gets an isolated temp.dir to work in, that my result in two
> JVMs sharing the same directory.
> Also, on Windows, if you call getTempDir on an existing directory, you get an
> "Access is denied" exception.
> Dawid proposed a simple solution to just call mkdirs() continuously until
> success. I'd like to try that.
> Also, I think that genTempFile could use some house cleaning, e.g.:
> * tempFileLocker can be just an Object instance? Why do we need a class?
> * If we initialize counter and counterBase in a static clause, we can avoid
> checking if counter==0 as well as passing Random to genTempFile (that will
> remove any suspicion that it does anything randomly)
> ** Also, instead of synchronizing on tempFileLocker, can we just use
> AtomicInteger for the counter?
> I'll modify getTempDir first. It documents "does not create the directory", I
> want to make sure no test fails due that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
