Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verifyInstrumentation warning about uninstrumented methods in Strand class #238

Open
cheese-stands-alone opened this issue Dec 5, 2016 · 10 comments

Comments

@cheese-stands-alone
Copy link

cheese-stands-alone commented Dec 5, 2016

Starting in quasar 0.7.7 the verifyInstrumentation warning about uninstrumented methods in Strand class, specifically sleep and park. I've created a sample project here https://github.com/rwhite226/quasar-verifyInstrumentation-test. When run with quasar 0.7.6 it runs normal, but when running with quasar 0.7.7 it outputs

WARNING: Uninstrumented whole methods ('**') or single calls ('!!') detected: at co.paralleluniverse.common.util.ExtendedStackTrace.here() (ExtendedStackTrace.java:44 bci: 8) at co.paralleluniverse.fibers.Fiber.checkInstrumentation() (Fiber.java:1668 bci: 0) at co.paralleluniverse.fibers.Fiber.verifySuspend(co.paralleluniverse.fibers.Fiber) (Fiber.java:1641 bci: 6) at co.paralleluniverse.fibers.Fiber.verifySuspend() (Fiber.java:1636 bci: 3) at co.paralleluniverse.fibers.Fiber.park(java.lang.Object,co.paralleluniverse.fibers.Fiber$ParkAction,long,java.util.concurrent.TimeUnit) (Fiber.java:620 bci: 0) at co.paralleluniverse.fibers.Fiber.park(java.lang.Object) (Fiber.java:632 bci: 4) at co.paralleluniverse.strands.Strand.park(java.lang.Object) (Strand.java:536 bci: 78) !! (instrumented suspendable calls at: BCIs [54], lines [536]) at co.paralleluniverse.strands.ConditionSynchronizer.await(int) (ConditionSynchronizer.java:54 bci: 255) at co.paralleluniverse.strands.channels.SingleConsumerQueueChannel.receive() (SingleConsumerQueueChannel.java:94 bci: 193) at co.paralleluniverse.actors.Actor.receive() (Actor.java:481 bci: 94) at com.test.Ping.doRun() (test.kt:29 bci: 356) at com.test.Ping.doRun() (test.kt:24 bci: 1) at co.paralleluniverse.actors.Actor.run0() (Actor.java:710 bci: 222) at co.paralleluniverse.actors.ActorRunner.run() (ActorRunner.java:51 bci: 148) at co.paralleluniverse.actors.Actor.run() (Actor.java:278 bci: 80) at co.paralleluniverse.fibers.Fiber.run() (Fiber.java:1072 bci: 11) at co.paralleluniverse.fibers.Fiber.run1() (Fiber.java:1067 bci: 1)

for each ping/pong.

@mikehearn
Copy link
Contributor

We see the same thing in Corda (github.com/corda/corda). We can't upgrade to 0.7.7 because it flags a call to Fiber.parkAndSerialize as uninstrumented. The issue appears to be a mismatch between the bytecode offset it's expecting to see and what it gets from inside HotSpot.

We have one method (annotated as @Suspendable by hand) that calls parkAndSerialize. It gets this BCI in the @Instrumented annotation:

suspendableCallSiteOffsetsAfterInstr = [112]

but what it got from the ExtendedStackTraceHotSpot$ExtendedStackTraceElement is 151 - they don't match and it gets flagged.

This code is grovelling around in HotSpot's memory, so it isn't surprising that this check is a bit fragile. Perhaps we could have a system property or some other flag to disable this code and fall back to just matching line numbers?

If I had to guess I'd point the finger at one of the JIT compilers doing some bytecode-level inlining which is then being reflected in the internal stack trace data.

@pron
Copy link
Contributor

pron commented Jan 4, 2017

What we can do is, instead of recording the bcis instrumented, we record the names of the method calls instrumented. This should be almost as accurate as bcis and far less brittle.

@mikehearn
Copy link
Contributor

Sounds good to me.

@pron
Copy link
Contributor

pron commented Jan 10, 2017

There's now a new name-based (rather than bci-based) verification technique 74c2fb4. You can give it a try at 0.7.8-SNAPSHOT.

@fab1an
Copy link

fab1an commented Mar 16, 2017

I have the same problem and tested with 0.7.8: #254

@circlespainter
Copy link
Member

@rwhite226 @mikehearn Can you give it a try again now with a fresh local 0.7.8-SNAPSHOT build from HEAD? The fix for #254 seems to have solved this one too for me, I've just tried with https://github.com/rwhite226/quasar-verifyInstrumentation-test and got no instrumentation warnings.

@vlad-poskatcheev-tealium

@circlespainter is there a planned release date for version 0.7.8?

@ov7a
Copy link

ov7a commented May 26, 2017

I've upgraded to 0.7.8 and still experiencing this issue.

@pron
Copy link
Contributor

pron commented Jul 31, 2017

Does the problem occur in 0.7.9?

@yanxinyuan
Copy link

yanxinyuan commented Mar 15, 2018

i occur this issue too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants