fix(runtime): Passing 'this' to a callback from constructor (#395)

In Javascript, a constructor is allowed to pass references to `this` to
method calls (effectively passing a partially initialized instance).
When done, the kernel will assign an Object ID to `this` on the spot in
order to pass the reference through the JSII boundary. However, the
`create` API before this change would have attempted to re-allocate an
Object ID to the object (which is illegal and causes a crash).

In addition, callbacks from at least Java and .NET runtimes could not
receive JSII object references, for they would not turn them into native
objects before passing them to the implementation, resulting in a class
conversion error.

.gitignore

@@ -3,4 +3,5 @@ node_modules/
 lerna-debug.log
 .DS_Store
 .idea
+.vs
 /dist

packages/jsii-calc/lib/compliance.ts

@@ -1583,3 +1583,20 @@ export class ConsumersOfThisCrazyTypeSystem {
         return { a: obj.a, b: obj.b, c: obj.c };
     }
 }
+
+
+//
+// Ensure the JSII kernel can pass "this" out to JSII remotes from within the constructor (this is dirty, but possible)
+///
+export abstract class PartiallyInitializedThisConsumer {
+    public abstract consumePartiallyInitializedThis(obj: ConstructorPassesThisOut, dt: Date, ev: AllTypesEnum): string;
+}
+
+export class ConstructorPassesThisOut {
+    constructor(consumer: PartiallyInitializedThisConsumer) {
+        const result = consumer.consumePartiallyInitializedThis(this, new Date(0), AllTypesEnum.ThisIsGreat);
+        if (result !== 'OK') {
+            throw new Error(`Expected OK but received ${result}`);
+        }
+    }
+}

packages/jsii-calc/package.json

@@ -36,11 +36,13 @@
   ],
   "dependencies": {
     "@scope/jsii-calc-base": "^0.8.0",
+    "@scope/jsii-calc-base-of-base": "^0.8.0",
     "@scope/jsii-calc-lib": "^0.8.0",
     "jsii-calc-bundled": "^0.8.0"
   },
   "peerDependencies": {
     "@scope/jsii-calc-base": "^0.8.0",
+    "@scope/jsii-calc-base-of-base": "^0.8.0",
     "@scope/jsii-calc-lib": "^0.8.0"
   },
   "devDependencies": {

packages/jsii-calc/test/assembly.jsii

@@ -84,6 +84,30 @@
       },
       "version": "0.8.0"
     },
+    "@scope/jsii-calc-base-of-base": {
+      "peer": true,
+      "targets": {
+        "dotnet": {
+          "namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace",
+          "packageId": "Amazon.JSII.Tests.CalculatorPackageId.BaseOfBasePackageId"
+        },
+        "java": {
+          "maven": {
+            "artifactId": "calculator-base-of-base",
+            "groupId": "software.amazon.jsii.tests"
+          },
+          "package": "software.amazon.jsii.tests.calculator.baseofbase"
+        },
+        "js": {
+          "npm": "@scope/jsii-calc-base-of-base"
+        },
+        "python": {
+          "distName": "scope.jsii-calc-base-of-base",
+          "module": "scope.jsii_calc_base_of_base"
+        }
+      },
+      "version": "0.8.0"
+    },
     "@scope/jsii-calc-lib": {
       "dependencies": {
         "@scope/jsii-calc-base": {
@@ -1250,6 +1274,23 @@
         }
       ]
     },
+    "jsii-calc.ConstructorPassesThisOut": {
+      "assembly": "jsii-calc",
+      "fqn": "jsii-calc.ConstructorPassesThisOut",
+      "initializer": {
+        "initializer": true,
+        "parameters": [
+          {
+            "name": "consumer",
+            "type": {
+              "fqn": "jsii-calc.PartiallyInitializedThisConsumer"
+            }
+          }
+        ]
+      },
+      "kind": "class",
+      "name": "ConstructorPassesThisOut"
+    },
     "jsii-calc.Constructors": {
       "assembly": "jsii-calc",
       "fqn": "jsii-calc.Constructors",
@@ -3327,6 +3368,45 @@
       ],
       "name": "OverrideReturnsObject"
     },
+    "jsii-calc.PartiallyInitializedThisConsumer": {
+      "abstract": true,
+      "assembly": "jsii-calc",
+      "fqn": "jsii-calc.PartiallyInitializedThisConsumer",
+      "initializer": {
+        "initializer": true
+      },
+      "kind": "class",
+      "methods": [
+        {
+          "abstract": true,
+          "name": "consumePartiallyInitializedThis",
+          "parameters": [
+            {
+              "name": "obj",
+              "type": {
+                "fqn": "jsii-calc.ConstructorPassesThisOut"
+              }
+            },
+            {
+              "name": "dt",
+              "type": {
+                "primitive": "date"
+              }
+            },
+            {
+              "name": "ev",
+              "type": {
+                "fqn": "jsii-calc.AllTypesEnum"
+              }
+            }
+          ],
+          "returns": {
+            "primitive": "string"
+          }
+        }
+      ],
+      "name": "PartiallyInitializedThisConsumer"
+    },
     "jsii-calc.Polymorphism": {
       "assembly": "jsii-calc",
       "fqn": "jsii-calc.Polymorphism",
@@ -4500,5 +4580,5 @@
     }
   },
   "version": "0.8.0",
-  "fingerprint": "WIFIhqgEwUDjCsDr7gliujhqcKZQSkDP+NstBfmdvZU="
+  "fingerprint": "J0bhm0cu1dlTAyMfBlAMWCVXZf6e8k2pHkmxye6P7Wk="
 }

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj

@@ -7,20 +7,20 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <PackageReference Include="Amazon.JSII.Tests.CalculatorPackageId" Version="$(JsiiVersion)"/>
+        <PackageReference Include="Amazon.JSII.Tests.CalculatorPackageId" Version="$(JsiiVersion)" />
     </ItemGroup>
 
     <ItemGroup>
-        <PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.0"/>
-        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0"/>
-        <PackageReference Include="Newtonsoft.Json" Version="11.0.2"/>
-        <PackageReference Include="xunit" Version="2.3.1"/>
-        <PackageReference Include="xunit.runner.visualstudio" Version="2.3.1"/>
-        <DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1"/>
+        <PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.0" />
+        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0" />
+        <PackageReference Include="Newtonsoft.Json" Version="11.0.2" />
+        <PackageReference Include="xunit" Version="2.3.1" />
+        <PackageReference Include="xunit.runner.visualstudio" Version="2.3.1" />
+        <DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
     </ItemGroup>
 
     <ItemGroup>
-        <ProjectReference Include="..\..\..\jsii-dotnet-runtime\src\Amazon.JSII.Runtime\Amazon.JSII.Runtime.csproj"/>
+        <ProjectReference Include="..\..\..\jsii-dotnet-runtime\src\Amazon.JSII.Runtime\Amazon.JSII.Runtime.csproj" />
     </ItemGroup>
 
 </Project>

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

@@ -892,10 +892,31 @@ public void EraseUnsetDataValues()
 
             Assert.True(EraseUndefinedHashValues.DoesKeyExist(opts, "option1"));
             Assert.False(EraseUndefinedHashValues.DoesKeyExist(opts, "option2"));
-            
+
             Assert.Equal(new Dictionary<string, object> { ["prop2"] = "value2" }, EraseUndefinedHashValues.Prop1IsNull());
             Assert.Equal(new Dictionary<string, object> { [ "prop1"] = "value1" }, EraseUndefinedHashValues.Prop2IsUndefined());
         }
+
+        [Fact(DisplayName = Prefix + nameof(ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut), Skip = "Currently broken")]
+        public void ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut()
+        {
+            var reflector = new PartiallyInitializedThisConsumerImpl();
+            var obj = new ConstructorPassesThisOut(reflector);
+
+            Assert.NotNull(obj);
+        }
+
+        class PartiallyInitializedThisConsumerImpl : PartiallyInitializedThisConsumer
+        {
+            public override String ConsumePartiallyInitializedThis(ConstructorPassesThisOut obj, DateTime dt, AllTypesEnum ev)
+            {
+                Assert.NotNull(obj);
+                Assert.Equal(new DateTime(0), dt);
+                Assert.Equal(AllTypesEnum.ThisIsGreat, ev);
+
+                return "OK";
+            }
+        }
         class NumberReturner : DeputyBase, IIReturnsNumber
         {
             public NumberReturner(double number)

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs

@@ -4,7 +4,9 @@
 using Amazon.JSII.Runtime.Deputy;
 using Amazon.JSII.Runtime.Services;
 using Amazon.JSII.Runtime.Services.Converters;
+using Newtonsoft.Json.Linq;
 using System;
+using System.Linq;
 using System.Reflection;
 
 namespace Amazon.JSII.Runtime
@@ -71,11 +73,11 @@ static CallbackResult InvokeMethod(InvokeRequest request, IReferenceMap referenc
 
             if (methodInfo == null)
             {
-                throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} getter, but this method does not exist");
+                throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} method, but this method does not exist");
             }
 
             JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
-            return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments));
+            return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments.Select(arg => FromKernel(arg, referenceMap)).ToArray()));
         }
 
         static CallbackResult InvokeGetter(GetRequest request, IReferenceMap referenceMap)
@@ -117,7 +119,24 @@ static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} setter, but this property does not have a setter");
             }
 
-            methodInfo.Invoke(deputy, new object[] { request.Value });
+            methodInfo.Invoke(deputy, new object[] { FromKernel(request.Value, referenceMap) });
+        }
+
+        /*
+         * This is a temporary workaround / hack to solve an immediate problem, but does not completely solve the
+         * problem to it's full extent. See https://github.com/awslabs/jsii/issues/404 for more information.
+         */
+        private static object FromKernel(object obj, IReferenceMap referenceMap)
+        {
+            if (obj is JObject)
+            {
+                var prop = ((JObject)obj).Property("$jsii.byref");
+                if (prop != null)
+                {
+                    return referenceMap.GetOrCreateNativeReference(new ByRefValue(prop.Value.Value<String>()));
+                }
+            }
+            return obj;
         }
     }
 

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs

@@ -54,7 +54,7 @@ protected DeputyBase(DeputyProps props = null)
 
             Reference = new ByRefValue(response["$jsii.byref"]);
             IReferenceMap referenceMap = serviceProvider.GetRequiredService<IReferenceMap>();
-            referenceMap.AddNativeReference(Reference, this);
+            referenceMap.AddNativeReference(Reference, this, true);
 
             Override[] GetOverrides()
             {

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/IReferenceMap.cs

@@ -5,7 +5,7 @@ namespace Amazon.JSII.Runtime.Services
 {
     public interface IReferenceMap
     {
-        void AddNativeReference(ByRefValue reference, DeputyBase nativeReference);
+        void AddNativeReference(ByRefValue reference, DeputyBase nativeReference, bool force = false);
 
         DeputyBase GetOrCreateNativeReference(ObjectReference reference);
 

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/ReferenceMap.cs

@@ -16,9 +16,9 @@ public ReferenceMap(ITypeCache types)
             _types = types ?? throw new ArgumentNullException(nameof(types));
         }
 
-        public void AddNativeReference(ByRefValue reference, DeputyBase nativeReference)
+        public void AddNativeReference(ByRefValue reference, DeputyBase nativeReference, bool force)
         {
-            if (_references.ContainsKey(reference.Value))
+            if (_references.ContainsKey(reference.Value) && !force)
             {
                 throw new ArgumentException(
                     $"Cannot add reference for {reference.Value}: A reference with this name already exists",

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

@@ -8,6 +8,7 @@
 import software.amazon.jsii.tests.calculator.AbstractClassReturner;
 import software.amazon.jsii.tests.calculator.Add;
 import software.amazon.jsii.tests.calculator.AllTypes;
+import software.amazon.jsii.tests.calculator.AllTypesEnum;
 import software.amazon.jsii.tests.calculator.AsyncVirtualMethods;
 import software.amazon.jsii.tests.calculator.Calculator;
 import software.amazon.jsii.tests.calculator.CalculatorProps;
@@ -38,6 +39,7 @@
 import software.amazon.jsii.tests.calculator.NullShouldBeTreatedAsUndefined;
 import software.amazon.jsii.tests.calculator.NullShouldBeTreatedAsUndefinedData;
 import software.amazon.jsii.tests.calculator.NumberGenerator;
+import software.amazon.jsii.tests.calculator.PartiallyInitializedThisConsumer;
 import software.amazon.jsii.tests.calculator.Polymorphism;
 import software.amazon.jsii.tests.calculator.Power;
 import software.amazon.jsii.tests.calculator.PublicClass;
@@ -55,6 +57,7 @@
 import software.amazon.jsii.tests.calculator.lib.Number;
 import software.amazon.jsii.tests.calculator.lib.StructWithOnlyOptionals;
 import software.amazon.jsii.tests.calculator.lib.Value;
+import software.amazon.jsii.tests.calculator.ConstructorPassesThisOut;
 
 import java.io.IOException;
 import java.time.Instant;
@@ -1010,6 +1013,27 @@ public void eraseUnsetDataValues() {
         assertEquals("{prop1=value1}", EraseUndefinedHashValues.prop2IsUndefined().toString());
     }
 
+    @Test
+    public void objectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut() {
+        final PartiallyInitializedThisConsumer reflector = new PartiallyInitializedThisConsumerImpl();
+        final ConstructorPassesThisOut object = new ConstructorPassesThisOut(reflector);
+
+        assertTrue(object != null);
+    }
+
+    static class PartiallyInitializedThisConsumerImpl extends PartiallyInitializedThisConsumer {
+        @Override
+        public String consumePartiallyInitializedThis(final ConstructorPassesThisOut obj,
+                                                      final Instant dt,
+                                                      final AllTypesEnum en) {
+            assertNotNull(obj);
+            assertEquals(Instant.EPOCH, dt);
+            assertEquals(AllTypesEnum.ThisIsGreat, en);
+
+            return "OK";
+        }
+    }
+
     static class MulTen extends Multiply {
         public MulTen(final int value) {
             super(new Number(value), new Number(10));

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/JsiiClientTest.java

@@ -116,7 +116,7 @@ public void asyncMethodOverrides() {
         assertEquals("overrideMe", first.getInvoke().getMethod());
         assertEquals("myCookie", first.getCookie());
         assertEquals(1, first.getInvoke().getArgs().size());
-        assertEquals(10, first.getInvoke().getArgs().get(0));
+        assertEquals(JsiiObjectMapper.valueToTree(10), first.getInvoke().getArgs().get(0));
         assertEquals(obj.getObjId(), JsiiObjectRef.parse(first.getInvoke().getObjref()).getObjId());
 
         // now complete the callback with some override value
@@ -146,7 +146,7 @@ public void asyncMethodOverridesThrow() {
         assertEquals("overrideMe", first.getInvoke().getMethod());
         assertEquals("myCookie", first.getCookie());
         assertEquals(1, first.getInvoke().getArgs().size());
-        assertEquals(10, first.getInvoke().getArgs().get(0));
+        assertEquals(JsiiObjectMapper.valueToTree(10), first.getInvoke().getArgs().get(0));
         assertEquals(obj.getObjId(), JsiiObjectRef.parse(first.getInvoke().getObjref()).getObjId());
 
         // now complete the callback with an error
@@ -171,7 +171,7 @@ public void syncVirtualMethods() {
         jsiiRuntime.setCallbackHandler(callback -> {
             assertEquals(obj.getObjId(), JsiiObjectRef.parse(callback.getInvoke().getObjref()).getObjId());
             assertEquals("virtualMethod", callback.getInvoke().getMethod());
-            assertEquals(10, callback.getInvoke().getArgs().get(0));
+            assertEquals(JsiiObjectMapper.valueToTree(10), callback.getInvoke().getArgs().get(0));
             assertEquals("myCookie", callback.getCookie());
 
             // interact with jsii from inside the callback
@@ -226,8 +226,7 @@ public void staticMethods() {
      */
     @Test
     public void serializeViaJsiiToJsonIfExists() {
-        JsiiObjectMapper om = JsiiObjectMapper.instance;
-        JsonNode result = om.valueToTree(new JsiiSerializable() {
+        JsonNode result = JsiiObjectMapper.INSTANCE.valueToTree(new JsiiSerializable() {
             public JsonNode $jsii$toJson() {
                 ObjectNode node = JSON.objectNode();
                 node.set("foo", OM.valueToTree("bar"));

packages/jsii-java-runtime/package.json

@@ -8,7 +8,7 @@
   "scripts": {
     "gen": "/bin/bash ./generate.sh",
     "build": "tsc && npm run gen && cd project && mvn deploy -D altDeploymentRepository=local::default::file://${PWD}/../maven-repo",
-    "test": "echo 'Tests are performed by jsii-java-runtime-test'",
+    "test": "echo 'Tests are run as part of the build target'",
     "package": "package-java"
   },
   "devDependencies": {