+1 to get a *new* SPI for the allocation (ok if we test if definingService is an instanceof it and reuse the same instance but should stay split) +1 to port the logic of tomee to OWB around unsafe with new method handles if it does not trigger any warning by default (was the reason to bypass Unsafe constructor when defining service is set).
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> Le mer. 6 oct. 2021 à 14:25, Jean-Louis MONTEIRO <jeano...@gmail.com> a écrit : > For the sake of clarity here is our problem. > We want to support JDK 17 in TomEE. > > For our proxy creation, we were used to using Unsafe (like OWB and a lot > more). > We changed that to use a method handles lookup, but still from JDK 17+ it > does not work either. > We have a similar service ClassDefiner in TomEE where we do the switch > automatically to ClassLoader.defineClass when it's available to create the > proxy from the byte array. > > OWB does that using explicit configuration but overall it is the same. > Where it becomes different is after ... > > As soon as you have created the Class with the byte array, you somehow need > to instantiate it. > In TomEE, we still by default use Unsafe.allocateInstance because there is > no replacement for now and it is still working under JDK17. > > For OWB, if you switch to using ClassLoader.defineClass for JDK 17, then > the default constructor is used and Unsafe is totally bypassed. > > I'm not questioning the choice made, but the fact we need to be able to > override that behavior in TomEE at least. > We can't always use the default constructor. Using Unsafe.allocateInstance > won't call the default constructor. > > If we can override OWB default behavior, then CDI beans managed by OWB and > beans managed by TomEE will work the same way and users can switch from one > to the other without side effects. > > So functionally it's the same with my change. > I'm almost sure no one is creating it's own DefiningClassService > implementation but the user facing interface argument is acceptable. I'd go > with a default method in the interface or create an > InstanciatingClassService even though it's overkill in my opinion. > > The comments in the tests should have been removed. I first wanted to add a > test to reproduce the issue we had in TomEE, but actually > InterceptionOfBeanWithConstructorInjectionTest already shows that using > default constructor instead of Unsafe.allocateInstance breaks OWB itself. > It also breaks a couple of other things in TomEE like the security > extension. > > > > > > > Le mer. 6 oct. 2021 à 11:17, Romain Manni-Bucau <rmannibu...@gmail.com> a > écrit : > > > Hi JL, > > > > It looks weird because we already had a fallback to use the constructor - > > and BTW i'm not sure the commented part of the test should be. > > So this shouldn't help TomEE. > > > > Do you have a test where this change helps? > > > > side note: we likely don't want to break the SPI since it is an user > facing > > part. > > I saw you mentionned a default method but we should probably check we > need > > it at all before (not sure how tomee is different there on java 17 since > > the extension points were already set up IIRC). > > > > Happy to discuss on slack if it is easier - know mails can be complicated > > for such things ;). > > > > Romain Manni-Bucau > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > <https://rmannibucau.metawerx.net/> | Old Blog > > <http://rmannibucau.wordpress.com> | Github < > > https://github.com/rmannibucau> | > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > > < > > > https://www.packtpub.com/application-development/java-ee-8-high-performance > > > > > > > > > Le mer. 6 oct. 2021 à 10:14, Jean-Louis MONTEIRO <jeano...@gmail.com> a > > écrit : > > > > > Thanks Thomas > > > > > > I've created https://issues.apache.org/jira/browse/OWB-1392 > > > And I pushed > > > > > > > > > https://github.com/apache/openwebbeans/commit/2af6184ee5ec6b474f037b3c5768c82bba136722 > > > > > > I'd appreciate feedback, review and comments. Should have created a PR > > > sorry. > > > Functionally, it's the same as previously, but it allows TomEE to > > override > > > the instanciation part to be consistent. > > > > > > > > > Le mar. 5 oct. 2021 à 23:11, Thomas Andraschko < > > > andraschko.tho...@gmail.com> > > > a écrit : > > > > > > > AFAIK we didnt start the process yet, so we can wait for your fix > > > > > > > > Am Di., 5. Okt. 2021 um 22:27 Uhr schrieb Jean-Louis MONTEIRO < > > > > jeano...@gmail.com>: > > > > > > > > > I have an issue with OWB in TomEE under JDK 17 > > > > > I think I can workaround it, but I'd need a small change in OWB. > > > > > > > > > > Can we reroll it after my fix? > > > > > > > > > > Le lun. 4 oct. 2021 à 09:29, Jean-Baptiste Onofré <j...@nanthrax.net > > > > a > > > > > écrit : > > > > > > > > > > > +1 > > > > > > > > > > > > Regards > > > > > > JB > > > > > > > > > > > > On 03/10/2021 20:56, Romain Manni-Bucau wrote: > > > > > > > Hi all, > > > > > > > > > > > > > > We fixed a few issues: > > > > > > > > > > > > > > PTKeySummaryAssigneeStatus > > > > > > > [image: Major] [image: Bug] OWB-1298 > > > > > > > <https://issues.apache.org/jira/browse/OWB-1298> > > > > WebsocketUserManager > > > > > > > ambigious resolution Jakarta Faces > > > > > > > <https://issues.apache.org/jira/browse/OWB-1298> Unassigned > > > RESOLVED > > > > > > > [image: Major] [image: Bug] OWB-1387 > > > > > > > <https://issues.apache.org/jira/browse/OWB-1387> > > > > > > > @Destroyed(ApplicationScoped.class) > > > > > > > not thrown when @Destroyed(RequestScoped.class) exists > > > > > > > <https://issues.apache.org/jira/browse/OWB-1387> Arne Limburg > > > > > > > < > > https://issues.apache.org/jira/secure/ViewProfile.jspa?name=arne> > > > > > > CLOSED > > > > > > > [image: Major] [image: Improvement] OWB-1389 > > > > > > > <https://issues.apache.org/jira/browse/OWB-1389> Remove > > destroyed > > > > > > instance > > > > > > > from memory <https://issues.apache.org/jira/browse/OWB-1389> > > Mark > > > > > > Struberg > > > > > > > < > > > > https://issues.apache.org/jira/secure/ViewProfile.jspa?name=struberg > > > > > > > > > RESOLVED > > > > > > > [image: Major] [image: Task] OWB-1390 > > > > > > > <https://issues.apache.org/jira/browse/OWB-1390> support > > > > > > > javax.enterprise.inject.scan.implicit property > > > > > > > <https://issues.apache.org/jira/browse/OWB-1390> Romain > > > Manni-Bucau > > > > > > > < > > > > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/secure/ViewProfile.jspa?name=romain.manni-bucau > > > > > > > > > > > > > > RESOLVED > > > > > > > [image: Major] [image: Task] OWB-1391 > > > > > > > <https://issues.apache.org/jira/browse/OWB-1391> > > > > > > AbstractMetaDataDiscovery > > > > > > > ignores classpath entries starting with a common path > > > > > > > <https://issues.apache.org/jira/browse/OWB-1391> Romain > > > Manni-Bucau > > > > > > > < > > > > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/secure/ViewProfile.jspa?name=romain.manni-bucau > > > > > > > > > > > > > > RESOLVED > > > > > > > > > > > > > > I know Thomas can await a few of them so wonder if we should > > > trigger > > > > a > > > > > > > release next week (starting on the 4th) or in the following > days. > > > > > > > > > > > > > > I'd just like to highlight the 1391 changes the way we ignore > > > > > duplicated > > > > > > > jars/folders in in the classpath so can be worth some testing. > > > > > > > > > > > > > > No issue to delay from some days the release if it helps. > > > > > > > > > > > > > > Side note for our beloved tomee siblings: this shouldn't impact > > you > > > > > since > > > > > > > you don't reuse that scanning/lifecycle logic so should be a > > "noop > > > > > > release" > > > > > > > for you. > > > > > > > > > > > > > > Romain Manni-Bucau > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog > > > > > > > <http://rmannibucau.wordpress.com> | Github < > > > > > > https://github.com/rmannibucau> | > > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > > > > > > > < > > > > > > > > > > > > > > > > > > > > > https://www.packtpub.com/application-development/java-ee-8-high-performance > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Jean-Louis > > > > > > > > > > > > > > > > > > -- > > > Jean-Louis > > > > > > > > -- > Jean-Louis >
