puniverse / quasar

Fibers, Channels and Actors for the JVM
http://docs.paralleluniverse.co/quasar/
Other
4.56k stars 574 forks source link

Instrumentation verification does not report some problems with lambda! #234

Closed victornoel closed 7 years ago

victornoel commented 7 years ago

Hi,

Initially reported on the mailing-list. A case where a method wasn't declared in suspendable-supers, instrumentation didn't happen for a lambda implementing it and the verification didn't report the missing instrumentation!

Basically, I have a method like this:

private <R, T extends WorkspaceRequest<R>> void answer(T r, CheckedFunction1<T, Either<Status, R>> f)
            throws SuspendExecution {
        try {
            if (w.users.stream().anyMatch(r.user::equals)) {
                RequestReplyHelper.reply(r, f.apply(r));
            } else {
                RequestReplyHelper.reply(r, Either.left(Status.FORBIDDEN));
            }
        } catch (Throwable e) {
            RequestReplyHelper.replyError(r, e);
        }
    }

CheckedFunction1 comes from the javaslang library and is thus not annotated.

And calling, for example, answer like this (handleImportBus being a suspendable method):

answer((ImportBus) msg, this::handleImportBus);

I hope it is clear enough to find the reason of the problem :)

pron commented 7 years ago

I couldn't quite reproduce this issue, but we've made some improvements to verifyInstrumentation reporting in 0.7.7 (released). Can you try with the new version?

victornoel commented 7 years ago

@pron nop, still nothing reported by verifyInstrumentation. I will try to make a simpler example!

victornoel commented 7 years ago

@pron here is a simpler example. QuasarTest1 shows when verifyInstrumentation works, and QuasarTest2 shows when it doesn't work. The second is maybe more complex that it needs to be… I used FiberAsync.runBlocking to be sure things will fails in case of instrumentation error, because if not quasar manages to run things without problems even if some stuffs are not instrumented it seems.

package test;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.function.Function;

import co.paralleluniverse.fibers.Fiber;
import co.paralleluniverse.fibers.FiberAsync;
import co.paralleluniverse.fibers.SuspendExecution;

public class QuasarTest1 {

  private static final ExecutorService exec = Executors.newSingleThreadExecutor();

  public static void main(String[] args) {

    new Fiber<>(() -> {
      test(s -> {
        try {
          return FiberAsync.runBlocking(exec, () -> s + "hop");
        } catch (RuntimeException | SuspendExecution | InterruptedException e) {
          throw new AssertionError(e);
        }
      });
    }).start();
  }

  private static void test(Function<String, String> f) throws SuspendExecution, InterruptedException {
    try {
      Fiber.sleep(100);
      System.out.println(f.apply("hop"));
    } catch (Throwable e) {
      Fiber.sleep(100);
      System.err.println(e.getMessage());
    }
  }
}
package test;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.function.Function;

import co.paralleluniverse.actors.ActorRef;
import co.paralleluniverse.actors.BasicActor;
import co.paralleluniverse.actors.behaviors.RequestMessage;
import co.paralleluniverse.actors.behaviors.RequestReplyHelper;
import co.paralleluniverse.fibers.FiberAsync;
import co.paralleluniverse.fibers.SuspendExecution;

public class QuasarTest2 {

  private static final ExecutorService exec = Executors.newSingleThreadExecutor();

  @SuppressWarnings("resource")
  public static void main(String[] args) throws InterruptedException, SuspendExecution {

    ActorRef<ActorMsg> r = new Actor().spawn();
    String res = RequestReplyHelper.call(r, new ActorMsg("hop"));
    System.out.println(res);

  }

  private static void test(ActorMsg m, Function<String, String> f) throws SuspendExecution {
    try {
      RequestReplyHelper.reply(m, f.apply(m.msg));
    } catch (Throwable e) {
      RequestReplyHelper.replyError(m, e);
    }
  }

  public static class ActorMsg extends RequestMessage<String> {

    private static final long serialVersionUID = -7392694803469583029L;

    final String msg;

    public ActorMsg(String msg) {
      this.msg = msg;
    }
  }

  public static class Actor extends BasicActor<ActorMsg, Void> {

    private static final long serialVersionUID = -4117919776467630960L;

    @Override
    protected Void doRun() throws InterruptedException, SuspendExecution {
      for (;;) {
        ActorMsg m = receive();
        test(m, s -> {
          try {
            return FiberAsync.runBlocking(exec, () -> s + "hop");
          } catch (RuntimeException | SuspendExecution | InterruptedException e) {
            throw new AssertionError(e);
          }
        });
      }
    }
  }
}
pron commented 7 years ago

Your first example doesn't even compile for me (I haven't tried the second yet). What precise version of Java are you using?

victornoel commented 7 years ago

I'm using java 8 in Eclipse, and yes, you are right, actually it only compiles with the eclipse compiler.

I already witnessed the problem before and forgot about it when submitting this issue. The problem is that type inference fails for FiberAsync.runBlocking(exec, () -> s + "hop"); Simply replace it with an explicit anonymous instantiation of CheckedCallable like this:

            return FiberAsync.runBlocking(exec, new CheckedCallable<String, RuntimeException>() {
              @Override
              public String call() throws RuntimeException {
                return s + "hop";
              }
            });
pron commented 7 years ago

Hmm, running QuasarTest2 with Quasar 0.7.7, I'm getting:

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.FiberAsync.run() (FiberAsync.java:118 bci: 92)
    at co.paralleluniverse.fibers.FiberAsync.runBlocking(java.util.concurrent.ExecutorService,co.paralleluniverse.common.util.CheckedCallable) (FiberAsync.java:409 bci: 113)
    at co.paralleluniverse.quasartkb.QuasarTest2$Actor.lambda$doRun$0(java.lang.String) (QuasarTest2.java:61 bci: 146)
    at co.paralleluniverse.quasartkb.QuasarTest2.test(co.paralleluniverse.quasartkb.QuasarTest2$ActorMsg,java.util.function.Function) (QuasarTest2.java:34 bci: 58) !! (instrumented suspendable calls at: BCIs [129, 222], lines [34, 36])
    at co.paralleluniverse.quasartkb.QuasarTest2.access$000(co.paralleluniverse.quasartkb.QuasarTest2$ActorMsg,java.util.function.Function) (QuasarTest2.java:19 bci: 2) (optimized)
    at co.paralleluniverse.quasartkb.QuasarTest2$Actor.doRun() (QuasarTest2.java:59 bci: 144)
    at co.paralleluniverse.quasartkb.QuasarTest2$Actor.doRun() (QuasarTest2.java:51 bci: 1) (optimized)
    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.fibers.Fiber.run() (Fiber.java:1072 bci: 11)
    at co.paralleluniverse.fibers.Fiber.run1() (Fiber.java:1067 bci: 1)
victornoel commented 7 years ago

Not sure why... I tried in eclipse and then from command line and it didn't work! Here is the same project exactly as I use it: test-quasar.tar.gz.

Decompress it and run it like so to see the problem I see (I hope :):

JAVA_HOME=/usr/lib/jvm/java-8-openjdk mvn clean compile dependency:properties exec:exec@run

I am using openjdk version "1.8.0_112"

pron commented 7 years ago

There's now a new verification technique 74c2fb42bd8f71fc21e4afd6e2f502f2f3c52550. You can give it a try at 0.7.8-SNAPSHOT.

victornoel commented 7 years ago

This time it works yes :) Thanks!

victornoel commented 7 years ago

@pron by the way, in these examples, I don't remember exactly my original problem, but how would you solve the instrumentation problem? By marking java.util.function.Function.apply(T) in suspendable-supers? Shouldn't quasar work with lambda automatically?

pron commented 7 years ago

The problem is that both the method itself and its call site (assuming the caller method is instrumented) must be instrumented. From the bytecode's perspective, this is just a call to Function.apply. Unless it's a suspendable-super (i.e. possibly has blocking implementations), Quasar doesn't know the call site must be instrumented. So the answer to your question is yes, but perhaps it's best to use a different interface for blocking calls in the first place.