Skip to content

Generate a correct proxy for interfaces #1728

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

Merged

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Jun 3, 2018

Static proxies generated for interfaces were incorrectly attempting to call the base "interface implementation", causing a PeVerify failure (but no runtime failure, the invalid code being on a never executed path). And they were all sharing the same type name.

(This flaw is also present in the dynamic proxy, but probably since its debut. Being now obsoleted, I do not intend to fix it.)

Static proxies generated for interfaces were incorrectly attempting to
call the base "interface implementation". And they were all sharing the
same type name.
@hazzik
Copy link
Member

hazzik commented Jun 7, 2018

What was the error?

Is it even possible to get null __lazyInitializer?

@hazzik
Copy link
Member

hazzik commented Jun 7, 2018

Ok, found the answer to my second question here:

// When deriving from the entity class, the entity class constructor may trigger
// virtual calls accessing the proxy state before its own constructor has a chance
// to initialize it. So although lazyInitializer is never supplied as null to the
// proxy constructor, we must guard nonetheless against it being null during base
// constructor call.

@fredericDelaporte
Copy link
Member Author

When I attempted re-implementing the lazy property proxy as a delegating proxy, I had to debug that trouble. Some tests where triggering it. I have not checked if removing that from the entity proxy would also cause some tests to start failing for the same reason. But since lazy entity proxies are used in many more tests than lazy property proxies, I am confident it would.

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Jun 8, 2018

What was the error?

The error fixed by this PR is invalid IL (failing PeVerify) generated for proxies on interface. These proxies were still usable, because the invalid code path was never executed (since in the interface case, there is no base constructor able of calling virtual members re-implemented by the proxy). But better generate correct proxies.

@fredericDelaporte fredericDelaporte merged commit 90e75b9 into nhibernate:master Jun 8, 2018
@fredericDelaporte fredericDelaporte deleted the StaticInterfaceProxy branch June 8, 2018 10:12
hazzik added a commit to hazzik/nhibernate-core that referenced this pull request Oct 24, 2018
Previously it was failing PEVerify similarly to nhibernate#1728 but for abstract classes (this is possible in case of a polymorphic entities). Unlike interfaces the abstract base class can go into a situation when lazy initializer is not yet available, eg. code in a constructor.
hazzik added a commit to hazzik/nhibernate-core that referenced this pull request Oct 25, 2018
Previously it was failing PEVerify similarly to nhibernate#1728 but for abstract classes (this is possible in case of a polymorphic entities). Unlike interfaces the abstract base class can go into a situation when lazy initializer is not yet available, eg. code in a constructor.
hazzik added a commit to hazzik/nhibernate-core that referenced this pull request Oct 25, 2018
Previously it was failing PEVerify similarly to nhibernate#1728 but for abstract classes (this is possible in case of a polymorphic entities). Unlike interfaces the abstract base class can go into a situation when lazy initializer is not yet available, eg. code in a constructor.
fredericDelaporte pushed a commit that referenced this pull request Oct 27, 2018
#1884)

Previously it was failing PEVerify similarly to #1728 but for abstract classes (this is possible in case of a polymorphic entities). Unlike interfaces the abstract base class can go into a situation when lazy initializer is not yet available, eg. code in a constructor.

Also test the explicit interface case, check access into constructor, and check proxy on type with
interfaces.

And correctly call explicit interface methods of base type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants