RFR(S) : 8029381 : assert(is_method_type()) failed: bad cast

[John Rose]

Wow, that hurts.  Thanks for tracking and fixing that bug, which I think was mine.

Safe publication is hard enough through a single pointer; it's nearly impossible when two or more words of racy state are involved.

Did you consider guarding more (or all) uses of _flags?  That would be a more disruptive change, of course.

I mean something like (but more complex than):
  intx flags()                     { return is_f1_null() ? 0 : _flags; }

Since CPCE is a highly-wrought racy union (the very model of a modern one) that version of flags wouldn't be appropriate for all typestates.

I agree with Vladimir marking the bug as hard to regression test.

— John

On May 30, 2014, at 1:36 PM, David Chase <david.r.chase at oracle.com> wrote:

> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8029381
> webrev: http://cr.openjdk.java.net/~drchase/8029381/webrev.00/
> 
> Problem: about once per month this assertion would fail in SQE testing,
> usually running one of the streams-related tests with -Xcomp.
> 
>  ciMethodType* as_method_type() {
>    assert(is_method_type(), "bad cast");
>    return (ciMethodType*)this;
>  }
> 
> The cause was a failure to check "f1" before accessing constant pool
> cache entries, and when competing threads raced, junk would be observed.
> This would only matter for invokedynamic -- the combo of needing both
> invokedynamic and multiple threads racing in the cpcache meant that 
> it was observed in the streams tests (even a test hand-crafted to make this
> failure occur would only fire about once ever 5000 times, and with -Xcomp that
> meant this was painfully slow).
> 
> The fix is to insert the check into the access macros themselves
> (for both method_type and appendix -- same problem for invokedynamic)
> and to remove the now-redundant checks from those places where it had
> been done right already.
> 
> Testing:
> 
> that bug was fixed: distilled the bug trigger out of the streams tests into something that
> did nothing by fork-joins into bug triggers.  Then modified hotspot to repeat one side of
> the race 1000 times in the event of an invokedynamic.  Observed failures once per 50 runs
> in this case, applied fix, observed no failures in 2600 runs.
> 
> that no harm was done:
> jtreg of hotspot/compiler
> jtreg of jdk/{jdk,vm}, no flags and -Xcomp
> jprt of hotspot
> 
> Question -- should test test case be added?  It practically never fails even when the
> bug is unfixed; without tweaking hotspot, it tells you practically nothing at all.  Checking
> the fix took all night.