Skip to content

Commit 850f42b

Browse files
RomainMullerRomainMuller
authored
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.
1 parent 0a83d52 commit 850f42b

File tree

74 files changed

+1069
-286
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+1069
-286
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ node_modules/
33
lerna-debug.log
44
.DS_Store
55
.idea
6+
.vs
67
/dist

‎packages/jsii-calc/lib/compliance.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,3 +1583,20 @@ export class ConsumersOfThisCrazyTypeSystem {
15831583
return { a: obj.a, b: obj.b, c: obj.c };
15841584
}
15851585
}
1586+
1587+
1588+
//
1589+
// Ensure the JSII kernel can pass "this" out to JSII remotes from within the constructor (this is dirty, but possible)
1590+
///
1591+
export abstract class PartiallyInitializedThisConsumer {
1592+
public abstract consumePartiallyInitializedThis(obj: ConstructorPassesThisOut, dt: Date, ev: AllTypesEnum): string;
1593+
}
1594+
1595+
export class ConstructorPassesThisOut {
1596+
constructor(consumer: PartiallyInitializedThisConsumer) {
1597+
const result = consumer.consumePartiallyInitializedThis(this, new Date(0), AllTypesEnum.ThisIsGreat);
1598+
if (result !== 'OK') {
1599+
throw new Error(`Expected OK but received ${result}`);
1600+
}
1601+
}
1602+
}

‎packages/jsii-calc/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,13 @@
3636
],
3737
"dependencies": {
3838
"@scope/jsii-calc-base": "^0.8.0",
39+
"@scope/jsii-calc-base-of-base": "^0.8.0",
3940
"@scope/jsii-calc-lib": "^0.8.0",
4041
"jsii-calc-bundled": "^0.8.0"
4142
},
4243
"peerDependencies": {
4344
"@scope/jsii-calc-base": "^0.8.0",
45+
"@scope/jsii-calc-base-of-base": "^0.8.0",
4446
"@scope/jsii-calc-lib": "^0.8.0"
4547
},
4648
"devDependencies": {

‎packages/jsii-calc/test/assembly.jsii

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,30 @@
8484
},
8585
"version": "0.8.0"
8686
},
87+
"@scope/jsii-calc-base-of-base": {
88+
"peer": true,
89+
"targets": {
90+
"dotnet": {
91+
"namespace": "Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace",
92+
"packageId": "Amazon.JSII.Tests.CalculatorPackageId.BaseOfBasePackageId"
93+
},
94+
"java": {
95+
"maven": {
96+
"artifactId": "calculator-base-of-base",
97+
"groupId": "software.amazon.jsii.tests"
98+
},
99+
"package": "software.amazon.jsii.tests.calculator.baseofbase"
100+
},
101+
"js": {
102+
"npm": "@scope/jsii-calc-base-of-base"
103+
},
104+
"python": {
105+
"distName": "scope.jsii-calc-base-of-base",
106+
"module": "scope.jsii_calc_base_of_base"
107+
}
108+
},
109+
"version": "0.8.0"
110+
},
87111
"@scope/jsii-calc-lib": {
88112
"dependencies": {
89113
"@scope/jsii-calc-base": {
@@ -1250,6 +1274,23 @@
12501274
}
12511275
]
12521276
},
1277+
"jsii-calc.ConstructorPassesThisOut": {
1278+
"assembly": "jsii-calc",
1279+
"fqn": "jsii-calc.ConstructorPassesThisOut",
1280+
"initializer": {
1281+
"initializer": true,
1282+
"parameters": [
1283+
{
1284+
"name": "consumer",
1285+
"type": {
1286+
"fqn": "jsii-calc.PartiallyInitializedThisConsumer"
1287+
}
1288+
}
1289+
]
1290+
},
1291+
"kind": "class",
1292+
"name": "ConstructorPassesThisOut"
1293+
},
12531294
"jsii-calc.Constructors": {
12541295
"assembly": "jsii-calc",
12551296
"fqn": "jsii-calc.Constructors",
@@ -3327,6 +3368,45 @@
33273368
],
33283369
"name": "OverrideReturnsObject"
33293370
},
3371+
"jsii-calc.PartiallyInitializedThisConsumer": {
3372+
"abstract": true,
3373+
"assembly": "jsii-calc",
3374+
"fqn": "jsii-calc.PartiallyInitializedThisConsumer",
3375+
"initializer": {
3376+
"initializer": true
3377+
},
3378+
"kind": "class",
3379+
"methods": [
3380+
{
3381+
"abstract": true,
3382+
"name": "consumePartiallyInitializedThis",
3383+
"parameters": [
3384+
{
3385+
"name": "obj",
3386+
"type": {
3387+
"fqn": "jsii-calc.ConstructorPassesThisOut"
3388+
}
3389+
},
3390+
{
3391+
"name": "dt",
3392+
"type": {
3393+
"primitive": "date"
3394+
}
3395+
},
3396+
{
3397+
"name": "ev",
3398+
"type": {
3399+
"fqn": "jsii-calc.AllTypesEnum"
3400+
}
3401+
}
3402+
],
3403+
"returns": {
3404+
"primitive": "string"
3405+
}
3406+
}
3407+
],
3408+
"name": "PartiallyInitializedThisConsumer"
3409+
},
33303410
"jsii-calc.Polymorphism": {
33313411
"assembly": "jsii-calc",
33323412
"fqn": "jsii-calc.Polymorphism",
@@ -4500,5 +4580,5 @@
45004580
}
45014581
},
45024582
"version": "0.8.0",
4503-
"fingerprint": "WIFIhqgEwUDjCsDr7gliujhqcKZQSkDP+NstBfmdvZU="
4583+
"fingerprint": "J0bhm0cu1dlTAyMfBlAMWCVXZf6e8k2pHkmxye6P7Wk="
45044584
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,20 @@
77
</PropertyGroup>
88

99
<ItemGroup>
10-
<PackageReference Include="Amazon.JSII.Tests.CalculatorPackageId" Version="$(JsiiVersion)"/>
10+
<PackageReference Include="Amazon.JSII.Tests.CalculatorPackageId" Version="$(JsiiVersion)" />
1111
</ItemGroup>
1212

1313
<ItemGroup>
14-
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.0"/>
15-
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0"/>
16-
<PackageReference Include="Newtonsoft.Json" Version="11.0.2"/>
17-
<PackageReference Include="xunit" Version="2.3.1"/>
18-
<PackageReference Include="xunit.runner.visualstudio" Version="2.3.1"/>
19-
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1"/>
14+
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.0" />
15+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0" />
16+
<PackageReference Include="Newtonsoft.Json" Version="11.0.2" />
17+
<PackageReference Include="xunit" Version="2.3.1" />
18+
<PackageReference Include="xunit.runner.visualstudio" Version="2.3.1" />
19+
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
2020
</ItemGroup>
2121

2222
<ItemGroup>
23-
<ProjectReference Include="..\..\..\jsii-dotnet-runtime\src\Amazon.JSII.Runtime\Amazon.JSII.Runtime.csproj"/>
23+
<ProjectReference Include="..\..\..\jsii-dotnet-runtime\src\Amazon.JSII.Runtime\Amazon.JSII.Runtime.csproj" />
2424
</ItemGroup>
2525

2626
</Project>

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,10 +892,31 @@ public void EraseUnsetDataValues()
892892

893893
Assert.True(EraseUndefinedHashValues.DoesKeyExist(opts, "option1"));
894894
Assert.False(EraseUndefinedHashValues.DoesKeyExist(opts, "option2"));
895-
895+
896896
Assert.Equal(new Dictionary<string, object> { ["prop2"] = "value2" }, EraseUndefinedHashValues.Prop1IsNull());
897897
Assert.Equal(new Dictionary<string, object> { [ "prop1"] = "value1" }, EraseUndefinedHashValues.Prop2IsUndefined());
898898
}
899+
900+
[Fact(DisplayName = Prefix + nameof(ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut), Skip = "Currently broken")]
901+
public void ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut()
902+
{
903+
var reflector = new PartiallyInitializedThisConsumerImpl();
904+
var obj = new ConstructorPassesThisOut(reflector);
905+
906+
Assert.NotNull(obj);
907+
}
908+
909+
class PartiallyInitializedThisConsumerImpl : PartiallyInitializedThisConsumer
910+
{
911+
public override String ConsumePartiallyInitializedThis(ConstructorPassesThisOut obj, DateTime dt, AllTypesEnum ev)
912+
{
913+
Assert.NotNull(obj);
914+
Assert.Equal(new DateTime(0), dt);
915+
Assert.Equal(AllTypesEnum.ThisIsGreat, ev);
916+
917+
return "OK";
918+
}
919+
}
899920
class NumberReturner : DeputyBase, IIReturnsNumber
900921
{
901922
public NumberReturner(double number)

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
using Amazon.JSII.Runtime.Deputy;
55
using Amazon.JSII.Runtime.Services;
66
using Amazon.JSII.Runtime.Services.Converters;
7+
using Newtonsoft.Json.Linq;
78
using System;
9+
using System.Linq;
810
using System.Reflection;
911

1012
namespace Amazon.JSII.Runtime
@@ -71,11 +73,11 @@ static CallbackResult InvokeMethod(InvokeRequest request, IReferenceMap referenc
7173

7274
if (methodInfo == null)
7375
{
74-
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} getter, but this method does not exist");
76+
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Method} method, but this method does not exist");
7577
}
7678

7779
JsiiMethodAttribute attribute = methodInfo.GetCustomAttribute<JsiiMethodAttribute>();
78-
return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments));
80+
return new CallbackResult(attribute?.Returns, methodInfo.Invoke(deputy, request.Arguments.Select(arg => FromKernel(arg, referenceMap)).ToArray()));
7981
}
8082

8183
static CallbackResult InvokeGetter(GetRequest request, IReferenceMap referenceMap)
@@ -117,7 +119,24 @@ static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
117119
throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} setter, but this property does not have a setter");
118120
}
119121

120-
methodInfo.Invoke(deputy, new object[] { request.Value });
122+
methodInfo.Invoke(deputy, new object[] { FromKernel(request.Value, referenceMap) });
123+
}
124+
125+
/*
126+
* This is a temporary workaround / hack to solve an immediate problem, but does not completely solve the
127+
* problem to it's full extent. See https://github.com/awslabs/jsii/issues/404 for more information.
128+
*/
129+
private static object FromKernel(object obj, IReferenceMap referenceMap)
130+
{
131+
if (obj is JObject)
132+
{
133+
var prop = ((JObject)obj).Property("$jsii.byref");
134+
if (prop != null)
135+
{
136+
return referenceMap.GetOrCreateNativeReference(new ByRefValue(prop.Value.Value<String>()));
137+
}
138+
}
139+
return obj;
121140
}
122141
}
123142

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected DeputyBase(DeputyProps props = null)
5454

5555
Reference = new ByRefValue(response["$jsii.byref"]);
5656
IReferenceMap referenceMap = serviceProvider.GetRequiredService<IReferenceMap>();
57-
referenceMap.AddNativeReference(Reference, this);
57+
referenceMap.AddNativeReference(Reference, this, true);
5858

5959
Override[] GetOverrides()
6060
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Amazon.JSII.Runtime.Services
55
{
66
public interface IReferenceMap
77
{
8-
void AddNativeReference(ByRefValue reference, DeputyBase nativeReference);
8+
void AddNativeReference(ByRefValue reference, DeputyBase nativeReference, bool force = false);
99

1010
DeputyBase GetOrCreateNativeReference(ObjectReference reference);
1111

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ public ReferenceMap(ITypeCache types)
1616
_types = types ?? throw new ArgumentNullException(nameof(types));
1717
}
1818

19-
public void AddNativeReference(ByRefValue reference, DeputyBase nativeReference)
19+
public void AddNativeReference(ByRefValue reference, DeputyBase nativeReference, bool force)
2020
{
21-
if (_references.ContainsKey(reference.Value))
21+
if (_references.ContainsKey(reference.Value) && !force)
2222
{
2323
throw new ArgumentException(
2424
$"Cannot add reference for {reference.Value}: A reference with this name already exists",

0 commit comments

Comments
 (0)