puniverse / quasar

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

Regression: 0.7.8 verification failing on method declared in suspendable-supers #279

Open victornoel opened 7 years ago

victornoel commented 7 years ago

Hi,

It worked well before, but since 0.7.8, with the following code:

    private <R, T extends CockpitRequest<R>> void answer(T r, CheckedFunction1<T, R> f) throws SuspendExecution {
        try {
            RequestReplyHelper.reply(r, f.apply(r));
        } catch (Throwable e) {
            RequestReplyHelper.replyError(r, e);
        }
    }

And the following in my META-INF/suspendable-supers:

javaslang.CheckedFunction1.apply

I get this instrumentation error:

! at org.ow2.petals.cockpit.server.actors.WorkspaceActor.answer(org.ow2.petals.cockpit.server.actors.CockpitActors$CockpitRequest,javaslang.CheckedFunction1) (WorkspaceActor.java:209) !! (instrumented suspendable calls at: lines [209, 209, 211],  calls [co.paralleluniverse.actors.behaviors.RequestReplyHelper.reply(co.paralleluniverse.actors.behaviors.RequestMessage, java.lang.Object), co.paralleluniverse.actors.behaviors.RequestReplyHelper.replyError(co.paralleluniverse.actors.behaviors.RequestMessage, java.lang.Throwable), javaslang.CheckedFunction1.apply(java.lang.Object)])

I'm not sure if it is the same as #275...

circlespainter commented 7 years ago

Hi, I might be not reproducing your case accurately but this one works OK for me in 0.7.8 with testgrp.ChkFun1.apply as the only line in META-INF/suspendable-supers (and the verification complains correctly if the line is not there):

import co.paralleluniverse.actors.ActorRef;
import co.paralleluniverse.actors.BasicActor;
import co.paralleluniverse.actors.LocalActor;
import co.paralleluniverse.actors.behaviors.RequestMessage;
import co.paralleluniverse.actors.behaviors.RequestReplyHelper;
import co.paralleluniverse.fibers.Fiber;
import co.paralleluniverse.fibers.SuspendExecution;
import co.paralleluniverse.fibers.Suspendable;

import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

interface ChkFun1<T, R> {
    R apply(T t);
}

class ChkFunSl<T, R> implements ChkFun1<T, R> {
    @Override
    @Suspendable
    public R apply(T t) {
        try {
            Fiber.sleep(1000);
        } catch (final InterruptedException e) {
            throw new RuntimeException(e);
        } catch (final SuspendExecution se) {
            throw new AssertionError(se);
        }
        return null;
    }
}

class RM extends RequestMessage<Object> {
    private final Object content;

    RM(Object content) {
        this.content = content;
    }

    public final Object getContent() {
        return content;
    }
}

class MyActor extends BasicActor<RM, Void> {
    @Override
    protected Void doRun() throws InterruptedException, SuspendExecution {
        final RM m = receive();
        final ChkFun1 h = new ChkFunSl<Void, Void>();
        System.out.println(h.apply(m));
        RequestReplyHelper.reply(m, new Object());
        return null;
    }
}

public final class Main {
    public static void main(String[] args) throws InterruptedException, ExecutionException, TimeoutException, SuspendExecution {
        final ActorRef<? super RM> ref = new MyActor().spawn();
        final Object ret = RequestReplyHelper.call(ref, new RM(new Object()));
        System.out.println(ret);
        LocalActor.join(ref, 10, TimeUnit.SECONDS);
    }
}

Can you provide a snippet or sample project that reproduces it, or give indications as to how to modify the above?

victornoel commented 7 years ago

deleted my previous comment, sorry, I completely forgot the META-INF file :P sorry, I will get back when I can reproduce my initial problem ^^

victornoel commented 7 years ago

Here is a reproductive example:

package test;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import co.paralleluniverse.actors.ActorRef;
import co.paralleluniverse.actors.BasicActor;
import co.paralleluniverse.actors.LocalActor;
import co.paralleluniverse.actors.behaviors.RequestMessage;
import co.paralleluniverse.actors.behaviors.RequestReplyHelper;
import co.paralleluniverse.fibers.Fiber;
import co.paralleluniverse.fibers.SuspendExecution;
import co.paralleluniverse.fibers.Suspendable;

interface ChkFun1<T, R> {
    R apply(T t);
}

class RM extends RequestMessage<Object> {
    private final Object content;

    RM(Object content) {
        this.content = content;
    }

    public final Object getContent() {
        return content;
    }
}

class MyActor extends BasicActor<RM, Void> {
    @Override
    protected Void doRun() throws InterruptedException, SuspendExecution {
        final RM m = receive();
        answer(m, this::doSomething);
        return null;
    }

    @Suspendable
    public Object doSomething(Object o) {
        System.out.println(o);
        try {
            Fiber.sleep(1000);
        } catch (final InterruptedException e) {
            throw new RuntimeException(e);
        } catch (final SuspendExecution se) {
            throw new AssertionError(se);
        }
        return null;
    }

    private <R, T extends RequestMessage<R>> void answer(T r, ChkFun1<T, R> f) throws SuspendExecution {
        try {
            RequestReplyHelper.reply(r, f.apply(r));
        } catch (Throwable e) {
            RequestReplyHelper.replyError(r, e);
        }
    }

}

public final class Main {
    public static void main(String[] args) throws InterruptedException, ExecutionException, TimeoutException, SuspendExecution {
        final ActorRef<? super RM> ref = new MyActor().spawn();
        final Object ret = RequestReplyHelper.call(ref, new RM(new Object()));
        System.out.println(ret);
        LocalActor.join(ref, 10, TimeUnit.SECONDS);
    }
}

I had to use a lambda I think to exhibit the problem!

circlespainter commented 7 years ago

Yes, it is indeed a regression from 0.7.7 to 0.7.8 introduced by name-based call-site instrumentation verification (https://github.com/puniverse/quasar/commit/24e9e5d8a331e4e924d5985fe5d2fcbae0faaf77#diff-4ef312b5913c12763d2e00ecfd5962f2) which, in its current form and in this case, doesn't work because the actual method call in the stack is provided through a method reference and has a different name from the interface method (and also a different owner with a completely separate hierarchy).

I'll need to think a bit about how this case can be covered.

circlespainter commented 7 years ago

It looks to me like, starting with lambdas, many more stack traces are possible than previously and specifically there are new traces where the caller calls into a functional interface call site and the runtime callee can be pretty much anything with a compatible desc, including methods with a different name and in a completely unrelated hierarchy.

We should probably be less demanding when checking that a functional interface call site is instrumented, that is allow as a callee anything with a desc compatible with one of the functional interface call sites we found during instrumentation. Of course the chance for false OKs would become bigger as we'd be less selective in this case, but maybe not by a lot if we manage to behave like this only for functional interfaces.

@pron What do you think?