Hello!
ReferenceQueue.remove(timeout) may return too early, i.e. before the
specified timeout has elapsed.
Would you please review the fix?
The change also includes a regression test, which can be used to
demonstrate the issue.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-6853696
WEBREV
Hi Ivan,
The code is correct as written but there might be some creep in the end
time due to the sampling of System.nanoTime.
I would be inclined to calculate the final time of the timeout once
and then compare simply with the current nanotime.
long end = (timeout == 0) ? Long.MAX_VALUE : (Sy
Thank you Roger for looking at the fix!
On 24.02.2014 18:37, roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the
end time due to the sampling of System.nanoTime.
I would be inclined to calculate the final time of the timeout once
and then compare s
On Feb 24 2014, at 06:37 , roger riggs wrote:
> Hi Ivan,
>
> The code is correct as written but there might be some creep in the end time
> due to the sampling of System.nanoTime.
>
> I would be inclined to calculate the final time of the timeout once
> and then compare simply with the curren
On 2/23/14 9:59 PM, Ivan Gerasimov wrote:
Hello!
ReferenceQueue.remove(timeout) may return too early, i.e. before the
specified timeout has elapsed.
Would you please review the fix?
The change also includes a regression test, which can be used to
demonstrate the issue.
BUGURL: https://bugs
On 25/02/2014 1:34 AM, Ivan Gerasimov wrote:
Thank you Roger for looking at the fix!
On 24.02.2014 18:37, roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the
end time due to the sampling of System.nanoTime.
I would be inclined to calculate the final
On 25.02.2014 6:52, David Holmes wrote:
On 25/02/2014 1:34 AM, Ivan Gerasimov wrote:
Thank you Roger for looking at the fix!
On 24.02.2014 18:37, roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the
end time due to the sampling of System.nanoTime.
On 25/02/2014 1:49 PM, Ivan Gerasimov wrote:
On 25.02.2014 6:52, David Holmes wrote:
On 25/02/2014 1:34 AM, Ivan Gerasimov wrote:
Thank you Roger for looking at the fix!
On 24.02.2014 18:37, roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the
end
Thank you Mike!
On 24.02.2014 22:26, Mike Duigou wrote:
On Feb 24 2014, at 06:37 , roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the end time
due to the sampling of System.nanoTime.
I would be inclined to calculate the final time of the timeout
Thank you Mandy!
On 25.02.2014 2:11, Mandy Chung wrote:
On 2/23/14 9:59 PM, Ivan Gerasimov wrote:
Hello!
ReferenceQueue.remove(timeout) may return too early, i.e. before the
specified timeout has elapsed.
Would you please review the fix?
The change also includes a regression test, which can
On 2/25/14 6:32 AM, Ivan Gerasimov wrote:
line 76: why do you want to catch InterruptedException? If
interrupted, should the test fail?
ReferenceQueue#remove() can throw InterruptedException, so I had to
handle it.
In the new webrev I set the initial value of actual to TIMEOUT, so if
the threa
Looks OK. Some minor comments:
- Some of the static fields in the test could be final (queue, weakReference,
startedSignal).
- After the join() loop in the test you could check that the weakReference is
cleared and enqueued.
- Why is the check thread.actual < TIMEOUT only done if thread.referen
Thanks you Mike!
On 25.02.2014 22:47, Mike Duigou wrote:
Looks OK. Some minor comments:
- Some of the static fields in the test could be final (queue, weakReference,
startedSignal).
Done.
- After the join() loop in the test you could check that the weakReference is
cleared and enqueued.
I
Thank you Mandy!
On 25.02.2014 21:53, Mandy Chung wrote:
On 2/25/14 6:32 AM, Ivan Gerasimov wrote:
line 76: why do you want to catch InterruptedException? If
interrupted, should the test fail?
ReferenceQueue#remove() can throw InterruptedException, so I had to
handle it.
In the new webrev I s
Hi Ivan,
143 long start = (timeout == 0) ? 0 : System.nanoTime();
This can simply be:
143 long start = System.nanoTime();
If timeout==0 then you will never execute the code that even looks at start.
Cheers,
David
On 26/02/2014 7:46 AM, Ivan Gerasimov wrote:
Thank y
Thanks David!
On 26.02.2014 8:44, David Holmes wrote:
Hi Ivan,
143 long start = (timeout == 0) ? 0 : System.nanoTime();
This can simply be:
143 long start = System.nanoTime();
If timeout==0 then you will never execute the code that even looks at
start.
That's ri
On 2/25/2014 1:46 PM, Ivan Gerasimov wrote:
line 61: I think the test should be:
if (thread.reference != null || thread.actual < TIMEOUT)
Sorry, I'm not clear why.
We have two threads:
1) The lucky one gets non-null reference when it calls remove(). For
this thread the actual time it ha
On 26.02.2014 10:19, Mandy Chung wrote:
On 2/25/2014 1:46 PM, Ivan Gerasimov wrote:
line 61: I think the test should be:
if (thread.reference != null || thread.actual < TIMEOUT)
Sorry, I'm not clear why.
We have two threads:
1) The lucky one gets non-null reference when it calls remove
On 2/25/2014 11:08 PM, Ivan Gerasimov wrote:
I missed that you remove the strong reference (line 57). I think
it's good to hold the strong reference so that
ReferenceQueue.remove(timeout) will timeout and the test can verify
reliably.
This is an important part.
If we didn't remove the stro
On 26.02.2014 22:43, Mandy Chung wrote:
On 2/25/2014 11:08 PM, Ivan Gerasimov wrote:
I missed that you remove the strong reference (line 57). I think
it's good to hold the strong reference so that
ReferenceQueue.remove(timeout) will timeout and the test can verify
reliably.
This is an im
On 2/26/2014 2:12 PM, Ivan Gerasimov wrote:
Good point, thank you Mandy. I should have added comments at the very
beginning.
Would you take a look at the last updated webrev, with the suggested
changes?
http://cr.openjdk.java.net/~igerasim/6853696/3/webrev/
thanks for making the change. It
21 matches
Mail list logo
