Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go: unable to reuse instances between child/parent interfaces #2688

Closed
1 of 5 tasks
iliapolo opened this issue Mar 14, 2021 · 1 comment · Fixed by #3321
Closed
1 of 5 tasks

go: unable to reuse instances between child/parent interfaces #2688

iliapolo opened this issue Mar 14, 2021 · 1 comment · Fixed by #3321
Assignees
Labels
bug This issue is a bug. compliance Missing or broken compliance tests effort/medium Medium work item – a couple days of effort language/go Regarding GoLang bindings p2

Comments

@iliapolo
Copy link
Contributor

🐛 Bug Report

When a TS child instance is first returned to the guest as a parent interface, the same instance cannot be returned as the child interface.

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Golang

General Information

  • JSII Version: ALL
  • Platform: ALL

What is the problem?

Consider the following TS code:

export class AnonymousImplementationProvider
implements IAnonymousImplementationProvider {
private readonly instance = new PrivateType();
public provideAsClass(): Implementation {
return this.instance;
}
public provideAsInterface(): IAnonymouslyImplementMe {
return this.instance;
}
}

Where:

class PrivateType extends Implementation implements IAnonymouslyImplementMe {

The following Golang code will work:

provider := calc.NewAnonymousImplementationProvider();
provider.ProvideAsClass(); // works - returning in instance of `Implementation`

But this subsequent invocation will fail:

provider.ProvideAsInterface(); // fails - trying to assign the same instance of `Implementation` to `IAnonymouslyImplementMe`
test panicked: reflect.Set: value of type *jsiicalc.jsiiProxy_Implementation is not assignable to type jsiicalc.IAnonymouslyImplementMe

Note that if the invocation order is reverse, it works. Because its now trying to assign a child to a parent, and not vise-versa.
Also, if the TS implementation returns a new instance on each Provide call, it will also work and since it won't reuse the previous instance.

Verbose Log

@iliapolo iliapolo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. compliance Missing or broken compliance tests language/go Regarding GoLang bindings p2 effort/medium Medium work item – a couple days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 14, 2021
@iliapolo iliapolo added this to Backlog in Go Developer Preview via automation Mar 14, 2021
@MrArnoldPalmer MrArnoldPalmer moved this from Backlog to Punt in Go Developer Preview Mar 18, 2021
@MrArnoldPalmer MrArnoldPalmer added this to Backlog in Go General Availability via automation Mar 30, 2021
@MrArnoldPalmer MrArnoldPalmer moved this from Backlog to To do in Go General Availability Apr 18, 2021
@RomainMuller RomainMuller self-assigned this Jan 5, 2022
RomainMuller added a commit that referenced this issue Jan 6, 2022
The go runtime was not tracking aliases in a way that enabled it to
return an alternate proxy to an object instance when that had previously
been returned as a different, singular interface.

This change keeps tack of the "first ever" registered instance against a
given InstanceID (as this is likely the one that was natively
constructed), but also to all proxy aliases to the same InstanceID, so
that it can later return an existing proxy that can be converted to a
desired type.

Fixes #2688
RomainMuller added a commit that referenced this issue Jan 6, 2022
The go runtime was not tracking aliases in a way that enabled it to
return an alternate proxy to an object instance when that had previously
been returned as a different, singular interface.

This change keeps tack of the "first ever" registered instance against a
given InstanceID (as this is likely the one that was natively
constructed), but also to all proxy aliases to the same InstanceID, so
that it can later return an existing proxy that can be converted to a
desired type.

Fixes #2688
RomainMuller added a commit that referenced this issue Jan 12, 2022
The go runtime was not tracking aliases in a way that enabled it to
return an alternate proxy to an object instance when that had previously
been returned as a different, singular interface.

This change keeps tack of the "first ever" registered instance against a
given InstanceID (as this is likely the one that was natively
constructed), but also to all proxy aliases to the same InstanceID, so
that it can later return an existing proxy that can be converted to a
desired type.

Fixes #2688
@mergify mergify bot closed this as completed in #3321 Jan 26, 2022
mergify bot pushed a commit that referenced this issue Jan 26, 2022
…3321)

The go runtime was not tracking aliases in a way that enabled it to
return an alternate proxy to an object instance when that had previously
been returned as a different, singular interface.

This change keeps tack of the "first ever" registered instance against a
given InstanceID (as this is likely the one that was natively
constructed), but also to all proxy aliases to the same InstanceID, so
that it can later return an existing proxy that can be converted to a
desired type.

Fixes #2688

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. compliance Missing or broken compliance tests effort/medium Medium work item – a couple days of effort language/go Regarding GoLang bindings p2
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants