Graal: 19.3 ToHostNodeGen targetTypeMapping caching breaks at generic_targetMapping_

[hashtag-smashtag]

We're seeing an issue with the js->java type mapping system in graal 19.3 where graal is incorrectly applying the wrong type mapper (explicit target type class does not match). It appears that:

• ToHostNodeGen (generated from ToHostNode) has 3 caching levels:cached0_cache, cached1_cache, and finally generic_targetMapping_
• The cached0_cache will hold up to 5 (ToHostNode.LIMIT) entries and then will transition to using cached1_cache instead
• The cached1_cache will hold up to 5 (ToHostNode.LIMIT) entries and then will transition to using generic_targetMapping_ instead
• The generic_targetMapping_ seems to only create 1 mapping and then incorrectly apply that to all subsequent mappings/conversions, even though the target type class does not match

What are some workarounds? Raising ToHostNode.LIMIT? Should ToHostNode#doGeneric even be using caching annotations in its method params?

Here's a test to reproduce, using testng:

package com.example.test;

import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.HostAccess;
import org.graalvm.polyglot.Value;
import org.testng.annotations.Test;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Date;

public class GraalIssues {

    // Graal 19.3 type mapping cache bug
    @Test
    void typeMappingCacheBug() {
        HostAccess hostAccess = HostAccess.newBuilder()
                .targetTypeMapping(Value.class, C1.class, null, C1::fromValue)
                .targetTypeMapping(Value.class, C2.class, null, C2::fromValue)
                .targetTypeMapping(Value.class, C3.class, null, C3::fromValue)
                .targetTypeMapping(Value.class, C4.class, null, C4::fromValue)
                .targetTypeMapping(Value.class, C5.class, null, C5::fromValue)
                .targetTypeMapping(Value.class, C6.class, null, C6::fromValue)
                .targetTypeMapping(Value.class, C7.class, null, C7::fromValue)
                .targetTypeMapping(Value.class, C8.class, null, C8::fromValue)
                .targetTypeMapping(Value.class, C9.class, null, C9::fromValue)
                .targetTypeMapping(Value.class, C10.class, null, C10::fromValue)
                .targetTypeMapping(Value.class, C11.class, null, C11::fromValue)
                .targetTypeMapping(Value.class, C12.class, null, C12::fromValue)
                .build();

        Context context = Context.newBuilder("js")
                .allowHostAccess(hostAccess)
                .build();

        // We'll be dealing with caches for node DynamicObjectBasic / DynamicObject<JSUserObject>
        Value value = context.eval("js", "let a = { someString: 'hi' }; a");

        // Fill up cached0_cache:
        value.as(C1.class);
        value.as(C2.class);
        value.as(C3.class);
        value.as(C4.class);
        value.as(C5.class);

        // Transition over to and fill up the cached1_cache:
        value.as(C6.class);
        value.as(C7.class);
        value.as(C8.class);
        value.as(C9.class);
        value.as(C10.class);

        // Transition over to generic_targetMapping_ and adds the C11 entry:
        value.as(C11.class);


        value = context.eval("js", "let b = { someDate: new Date() }; b");
        try {
            // Will erroneously use the C11 mapping in generic_targetMapping_ 
            value.as(C12.class);
            assert false;
        } catch (Throwable t) {
            StringWriter stringWriter = new StringWriter();
            t.printStackTrace(new PrintWriter(stringWriter));
            assert stringWriter.toString().contains("C11.fromValue");
        }
    }

    public static class C1 {
        public String someString;

        public static C1 fromValue(Value value) {
            C1 c1 = new C1();
            c1.someString = value.getMember("someString").asString();
            return c1;
        }
    }

    public static class C2 {
        public String someString;

        public static C2 fromValue(Value value) {
            C2 c2 = new C2();
            c2.someString = value.getMember("someString").asString();
            return c2;
        }
    }

    public static class C3 {
        public String someString;

        public static C3 fromValue(Value value) {
            C3 c3 = new C3();
            c3.someString = value.getMember("someString").asString();
            return c3;
        }
    }

    public static class C4 {
        public String someString;

        public static C4 fromValue(Value value) {
            C4 c4 = new C4();
            c4.someString = value.getMember("someString").asString();
            return c4;
        }
    }

    public static class C5 {
        public String someString;

        public static C5 fromValue(Value value) {
            C5 c5 = new C5();
            c5.someString = value.getMember("someString").asString();
            return c5;
        }
    }

    public static class C6 {
        public String someString;

        public static C6 fromValue(Value value) {
            C6 c6 = new C6();
            c6.someString = value.getMember("someString").asString();
            return c6;
        }
    }

    public static class C7 {
        public String someString;

        public static C7 fromValue(Value value) {
            C7 c7 = new C7();
            c7.someString = value.getMember("someString").asString();
            return c7;
        }
    }

    public static class C8 {
        public String someString;

        public static C8 fromValue(Value value) {
            C8 c8 = new C8();
            c8.someString = value.getMember("someString").asString();
            return c8;
        }
    }

    public static class C9 {
        public String someString;

        public static C9 fromValue(Value value) {
            C9 c9 = new C9();
            c9.someString = value.getMember("someString").asString();
            return c9;
        }
    }

    public static class C10 {
        public String someString;

        public static C10 fromValue(Value value) {
            C10 c10 = new C10();
            c10.someString = value.getMember("someString").asString();
            return c10;
        }
    }

    public static class C11 {
        public String someString;

        public static C11 fromValue(Value value) {
            C11 c11 = new C11();
            c11.someString = value.getMember("someString").asString();
            return c11;
        }
    }

    public static class C12 {
        public Date someDate;

        public static C12 fromValue(Value value) {
            C12 c12 = new C12();
            c12.someDate = value.getMember("someDate").as(Date.class);
            return c12;
        }
    }
}

Thanks.

[chumer]

Thanks a lot for this involved report. Unfortunately I don't have a good workaround. Maybe reducing the number of target type mappings could help?

[ianwixon]

Thank you for taking a look at this. In our use case we are unable to limit the number of registered type mappers - we require more than 10 mapped types.

A workaround I was able to successfully pass the provided test was to increase the ToHostNode.LIMIT to be large enough that all of the required mappers would fit in the inline cache. Quick testing seemed to show it wasn't appreciably slower using a small number of mappers (~30, not 300). Curious on your thoughts on a workaround such as this.

Another thing I noticed was that once ToHostNode decides to replace its doCached() method with doGeneric() that method uses an @Cached TargetMappingNode as one of its arguments. If my understanding of that annotation is correct, would it cache and reuse the argument it was first called with? https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ToHostNode.java#L126

[boris-spas]

Tracking internally as Issue GR-20948.

[chumer]

Fix is in the pipeline. Will be fixed in 20.1, backported to 20.0.1 and 19.3.2

[chumer]

Fix landed in master.
https://github.com/oracle/graal/commit/33e3b475818b5ca34ed517db8033a1195f567f61