RFR: 6900441 PlatformEvent.park(millis) on Linux could still be affected by changes to the time-of-day clock

David Holmes david.holmes at oracle.com
Thu Sep 12 14:47:32 PDT 2013


Hi Dan,

I found a bug in the assertion related logic - new webrev:

http://cr.openjdk.java.net/~dholmes/6900441/webrev.v3/

For the untimed case in park I wasn't setting the _cur_index value (I 
was implicitly using zero) and so the assertion it wasn't -1 was failing 
in unpark.

With regard to assert versus assert_status - yes this would work:

assert_status(status == 0, errno, "gettimeofday");

I had never considered that. :) When I added assert_status to diagnoze a 
particular issue I never attempted to change all the existing asserts on 
the library calls. So we have a lot of simple 
"assert(status==0,"invariant") where assert_status could have been used, 
as well as the old gettimeofday style calls. Do you want me to change 
them all in os_linux.cpp?

Thanks,
David

On 13/09/2013 1:14 AM, Daniel D. Daugherty wrote:
> On 9/11/13 5:14 PM, David Holmes wrote:
>> updated webrev:
>>
>> http://cr.openjdk.java.net/~dholmes/6900441/webrev.v2/
>
> Looks good. Possibly modulo one comment below...
>
>
>> On 12/09/2013 7:10 AM, Daniel D. Daugherty wrote:
>>
>>>      line 5541: assert_status(status == 0, status, "clock_gettime");
>>>      line 5553: assert(status == 0, "gettimeofday");
>>>          So why is one an assert_status() call and the other is a
>>>          plain old assert() call?
>>
>> Different API's. The old unix API's, like gettimeofday return -1 on
>> error and set errno. The "modern" posix APIs, eg pthread APIs and
>> clock_gettime etc, return the actual error code on error - hence
>> assert_status can be used to show the actual error in that case.
>
> I don't quite get what you're trying to say here.
> It seems that both calls are trying to verify
> that "status == 0". Or are you saying that:
>
>      assert_status(status == 0, status, "gettimeofday");
>
> is kind of a waste because "status" always be either "0" or "-1".
> So how about this:
>
>      assert_status(status == 0, errno, "gettimeofday");
>
> instead? That pattern should work to get more info.
>
> Dan
>


More information about the hotspot-dev mailing list