Oct 25 09:33:30 hey smarlow could you run through the list of reasons that simplistic SPI for JPA plugability doesnt work for hibernate and other JPA providers Oct 25 09:36:04 Nihility: I think the biggest SPI or lack of standard property, is the specifying of the TM or TSR Oct 25 09:36:35 smarlow: more specifically im wondering why the list of entities doesnt work with hibernate Oct 25 09:36:48 Nihility: ahh Oct 25 09:36:56 oh because its reflection based Oct 25 09:36:57 right? Oct 25 09:37:50 Nihility: well, I'm not sure exactly where it falls down, the Hibernate file reading code just quickly ends and doesn't find the entity classes. Oct 25 09:38:42 Nihility: I'll look for a git link Oct 25 09:43:17 Hasn't the problem always been "what's behind URL #1?" of getJarFileUrls and getPersistenceUnitRootUrl? http://download.oracle.com/javaee/6/api/javax/persistence/spi/PersistenceUnitInfo.html Oct 25 09:43:30 Nihility: https://github.com/hibernate/hibernate-core/tree/master/hibernate-entitymanager/src/main/java/org/hibernate/ejb/packaging has AbstractJarVisitor, FileZippedJarVisitor, InputStreamZippedJarVisitor and other code to deal reading files. Somewhere in there is a reason why getting the list of entities from a VFS file doesn't work. Oct 25 09:43:54 wolfc: yes, its a vfs url. Oct 25 09:44:38 yeah, it's really wicked. I tried some code in Shrinkwrap for a similar problem, but that code broke down. Oct 25 09:45:09 wolfc, Nihility: I assume we just want to understand why the problem exists, not solve it now? Since our annotation scanner works. Oct 25 09:45:14 yeah i dont get why it scans directories Oct 25 09:45:34 if we are giving a list of class file URLs Oct 25 09:45:59 then i imagine that should encompass every class visible to the deployment that has @Entity on it Oct 25 09:46:08 http://download.oracle.com/javaee/6/api/javax/persistence/spi/PersistenceUnitInfo.html#excludeUnlistedClasses%28%29 looks new Oct 25 09:46:17 nickarls, you around? Oct 25 09:46:19 i suppose the problem is stuff in persisentence.xml Oct 25 09:46:24 maeste: works like charm, thanks Oct 25 09:46:38 maeste: but make sure not to remove the "enabled" attribute Oct 25 09:46:42 smarlow: i am wondering if we can just fix this stupid spec by tweaking it for EE7, then we can stop maintaining so many jpa integration layers Oct 25 09:46:45 hbraun: yup, I'm doing Oct 25 09:46:56 people can in essence paste anything in it Oct 25 09:47:00 maeste: i mean _not_ to remove the attribute Oct 25 09:47:06 hbraun: why? Oct 25 09:47:12 maeste: somehow I need to read the current state Oct 25 09:47:31 maeste: how would I know if a DS is enabled otherwise? Oct 25 09:47:41 Nihility: I think we can make incremental improvements, others can argue against them. This scanning thing, could be solved with vfs changes perhaps Oct 25 09:47:42 yup, But I'm just removing it from operations, not resources Oct 25 09:47:51 ah, yes Oct 25 09:48:04 don't know all the details, but it sounds reasonaböe Oct 25 09:48:08 smarlow: scanning off of URLS is stupid Oct 25 09:48:09 Nihility: I think we should definitely make the TM/TSR integration via a standard property, part of the spec Oct 25 09:48:33 smarlow: the jpa provider shouldn't *look* for files unless there was some kind of spi for that Oct 25 09:49:26 yeah, I like Bernhards suggestion of explicitly listing the classes and then set excludeUnlisted to true Oct 25 09:50:17 wolfc: we do that for hibernate 3.3 integration currently, since we cannot use our custom annotation scanner (Hibernate 3.3 does't support the custom annotation scanner) Oct 25 09:50:34 wolfc: we don't, I mean the user does ;) Oct 25 09:50:48 they have to list each entity class in the persistence.xml Oct 25 09:52:36 Nihility: to fix our broken PersistenceUnitMetadata.getNewTempClassLoader() (AS7-886), I am wondering if that could be solved with reflection (I started going down that path but not sure if it will end well) Oct 25 09:52:38 jira [AS7-886] PersistenceUnitMetadata.getNewTempClassLoader() cannot load the entity classes [Open (Unresolved) Bug, Minor, Scott Marlow] https://issues.jboss.org/browse/AS7-886 Oct 25 09:53:22 Nihility: we need to (application level classes only) clone the current application classloader Oct 25 09:54:37 that method doesnt make any sense to me Oct 25 09:55:20 why have it over getClassLoader() Oct 25 09:55:21 JPA uses it for annotation scanning, then transforms the class and loads the transformed class with the real loader Oct 25 09:55:26 which should it return a different value Oct 25 09:55:50 Nihility: its basically supposed to return a temporary (good at deployment time only) classloader, that the entity classes can be loaded with, with out actually loading the entity classes on the real application class loader Oct 25 09:56:05 but why? Oct 25 09:56:39 why use a ClassTransformer? is that the question? Oct 25 09:56:56 no why have a different temporary than main if they see the same classes Oct 25 09:57:01 or why does it need transformation? Oct 25 09:57:09 so the provider can get the list of entity classes without interfering with its later entity class transformation Oct 25 09:57:25 pferraro: morning! your latest pull worked beautifully, but stuartdouglas had a few comments Oct 25 09:57:29 ah ok Oct 25 09:57:48 Hibernate doesn't need it but other providers expect it Oct 25 09:57:58 so its the original unmodified class? Oct 25 09:58:17 this still doesnt make sense Oct 25 09:58:22 JPA instruments without changing the class name Oct 25 09:58:50 getClassLoader() is assumed then to have a java.lang.instrument.transformer on it? Oct 25 09:59:01 http://download.oracle.com/javaee/6/api/javax/persistence/spi/ClassTransformer.html Oct 25 09:59:30 Nihility: yup, it expects to load the original classes, just to scan for jpa annotations, later, it will use the real classloader to do the (yes, using ClassTransformer which we also need to implement still) Oct 25 10:00:57 wow i cant even count the ways that this design blows Oct 25 10:01:02 :-) Oct 25 10:01:02 lets see Oct 25 10:01:14 I expect to get community help with ClassTransformer, which we don't need with Hibernate Oct 25 10:01:17 1) double loading classes Oct 25 10:01:31 2) weaving isnt even necessary to implement JPA if you are doing it you fucked up Oct 25 10:01:59 you need it for managed entity instances, not? Oct 25 10:02:22 no you just need to generate new backing implementations Oct 25 10:02:38 thats why hibernate does use it Oct 25 10:02:42 doesnt Oct 25 10:03:13 even if it did Oct 25 10:03:21 you should need to load the same class def twice Oct 25 10:03:28 which is the reflection bullshit way of analysis Oct 25 10:03:49 such an entity wouldn't be able to traverse CL boundaries, would it? Oct 25 10:05:25 hm I can't believe there was only one JIRA about remoting invalid message lengths and that sort of thing Oct 25 10:05:26 wolfc: not sure what you mean, you can generate subclassing proxies either in the same CL, or you can generate them in a different CL Oct 25 10:06:14 wolfc: thanks though for explaining why its done like that Oct 25 10:06:27 In essence an entity can leave the building Oct 25 10:06:34 at which point it becomes unmanaged Oct 25 10:06:55 then it can come back in, at which point it can become managed again at a specific Oct 25 10:07:19 smarlow: ok so this means that we need to create a new module dynamically based on the same resources as the applications module Oct 25 10:07:29 at least that's how I understand it Oct 25 10:07:38 smarlow: but there is ALOT of problems that come into play with this Oct 25 10:07:55 Nihility: dynamically via reflecting hacking or spi that we would add? Oct 25 10:09:02 smarlow: we would have to restructure our deployment modularity stuff Oct 25 10:09:33 smarlow: basically the module that is used in the deployment chain would be getTempClassLoader() Oct 25 10:09:54 smarlow: but at runtime it would have to switch to a new cloned module Oct 25 10:10:05 smarlow: one that has the transformer hook Oct 25 10:10:13 and that would be getClassLoader Oct 25 10:10:30 and then everything that depends on tha tmodule Oct 25 10:10:35 has to be refreshed Oct 25 10:10:39 so yeah its nasy Oct 25 10:12:41 * smarlow looking at http://community.jboss.org/message/633486#633486 to see how Antii hacked the transformer in for his testing with OpenJPA Oct 25 10:12:43 dmlloyd: you see teh above^ Oct 25 10:14:18 i suppose it should work Oct 25 10:15:06 the class redefinition needs to happen AFTER all DUPS look at the classes Oct 25 10:15:14 but BEFORE component install completes Oct 25 10:16:02 otherwise we might be able to do something clever Oct 25 10:16:12 like allow you to keep a module but recycle the classloader on it Oct 25 10:16:13 Antii appears to be adding his custom transformer (via reflection) to the current application classloader before OpenJPA scans entity classes for annotations I think Oct 25 10:19:00 yeah that idea might actually work Oct 25 10:19:07 need to run it by dmlloyd Oct 25 10:19:30 but we might be able to do something where we toss the original MCL, and construct a new one with the transformer Oct 25 10:19:55 all classes loaded before it will fail though so need to think about that Oct 25 10:24:55 Nihility: for the temp MCL and the MCL we switch to later. Could we share references to other modules? As long as we don't try to share entity classes between modules, I think the entity classes would be in the top level or one level down in a application sub-module. So, the two MCLs need to keep the application classes separate but not other module references (I think). If that makes sense, does it make it easier? Oct 25 10:27:23 smarlow: theres two ways we can go about it, one is we can redefine the module, which is messy and will require refreshing every module that refers to it Oct 25 10:28:09 smarlow: the other way, which may or may not work, is that we can add a feature to modules that allows you to recreate a fresh classloader, this (in theory) would not require mucking with any other modules Oct 25 10:28:34 smarlow: both however have to be done in the exact *right* time in the DUP chain Oct 25 10:29:09 smarlow: everything up until the component install (which potentially executes app code) needs to see the original loader Oct 25 10:30:53 Nihility: hmmm … where did standalone-preview.xml go? Oct 25 10:31:10 or what's it called now? Oct 25 10:31:48 alesj: standalone.xml has everything now Oct 25 10:32:04 jpederse: ah, ok Oct 25 10:32:11 need to fix this in Weld then ... Oct 25 10:32:42 Nihility: my preference would be to not switch the loaders, but instead create a fresh classloader for the temp loader. Clearly this will improve our plugability support but I question whether we do it now or in a later release. Not doing it now, is something I need to explore, if we chose that path. Oct 25 10:44:53 smarlow: it affects our SPIs and APIs so we need to at least consider it now Oct 25 10:48:16 bstansberry: ping Oct 25 10:51:22 bobmcw: no worries - its a regression that needed fixing regardless Oct 25 10:53:44 okay catching up on this Oct 25 10:57:46 I have no idea Oct 25 10:57:59 sounds like typical EE spec BS Oct 25 10:58:20 what exactly do we need and why? Oct 25 10:59:03 dmlloyd: we are running a compliance test with a 3rd party provider that tries to use the temp classloader Oct 25 11:01:01 that is the why, the what is a temporary application classloader, that can be used to load the entity classes for annotation scanning Oct 25 11:01:26 dmlloyd: without causing the real application classloader to actually load the same classes Oct 25 11:02:02 that's pretty weak Oct 25 11:02:12 do we need this for EAP 6.0? Oct 25 11:02:45 dmlloyd: we don't need it for Hibernate but do for TopLink Oct 25 11:02:47 are we sure we can't just transform the classes in the first place? Oct 25 11:03:16 is TopLink required for cert? Oct 25 11:04:13 jboss-modules does allow you to plug a (java.lang.instrument.)ClassFileTransformer in to the module when it's created Oct 25 11:04:23 dmlloyd: yes, there is a test that we need to pass with TopLink for cert. Its a grey area how much of the test we need to pass. Oct 25 11:04:47 if we can just grab the transformer in the first place we won't need the two stages of CL Oct 25 11:04:59 unless that nonsense is actually in the spec Oct 25 11:05:07 I don't think TopLInk needs the Transformer Oct 25 11:05:22 then why does it need two stages of CL Oct 25 11:05:35 why not just scan the permanent CL Oct 25 11:05:40 Transformer is a separate thing in the spec, that we also don't need for hibernate but OpenJPA uses it Oct 25 11:07:13 dmlloyd: here is the thing, If I let TopLink use the permanent CL to scan the entities, that breaks the API contract of PersistenceUnitMetadata.getNewTempClassLoader(), which is suppose to return a classloader that doesn't impact the application classloader Oct 25 11:08:33 I can't find that class, is it part of the spec? Oct 25 11:10:06 * smarlow its really PersistenceUnitInfo Oct 25 11:10:18 http://download.oracle.com/javaee/6/api/javax/persistence/spi/PersistenceUnitInfo.html Oct 25 11:10:37 dmlloyd: http://download.oracle.com/javaee/6/api/javax/persistence/spi/PersistenceUnitInfo.html#getNewTempClassLoader() Oct 25 11:11:08 I don't see the conflict Oct 25 11:11:24 toplink will surely choose the appropriate CL to do its scanning Oct 25 11:12:00 addTransformer() btw is really easy to implement, I can help with that when you get to it Oct 25 11:12:03 dmlloyd: well back to the question, I'm looking to use reflection I think, to implement getNewTempClassLoader. Oct 25 11:12:28 or if it should be done without reflection, let me know. Oct 25 11:12:44 dmlloyd: I started down the reflection path but didn't go all the way down yet Oct 25 11:12:58 dmlloyd: just wanted to check in if that was likely to work Oct 25 11:13:30 nah for getNewTempClassLoader() I'd just create a new ConcurrentClassLoader subclass whose parent is the module Oct 25 11:17:15 dmlloyd: the conflict is that the spec is shit Oct 25 11:17:17 :) Oct 25 11:17:32 dmlloyd: the jpa provider needs to load classes without its transformer being applied Oct 25 11:17:42 dmlloyd: probably to be able to do reflective analysis Oct 25 11:17:52 yeah the transformer clearly applies to classes defined, not loaded Oct 25 11:18:05 the spec is pretty clear here Oct 25 11:18:12 I don't think it'll be an issue Oct 25 11:18:31 dmlloyd: the Class is rewritten by the provider Oct 25 11:18:32 dmlloyd: something like https://github.com/scottmarlow/jboss-as/blob/master/jpa/core/src/main/java/org/jboss/as/jpa/classloader/TempClassLoader.java which is probably wrong in a different way? Is there a trick that i should use to use the parent? I also have https://github.com/scottmarlow/jboss-as/blob/classloader/jpa/core/src/main/java/org/jboss/as/jpa/classloader/TempClassLoader.java which uses the parent.getModule() but doesn't prevent the Oct 25 11:18:32 parent from loading the application classes (need to go deeper I think) Oct 25 11:18:36 dmlloyd: which is why it has 2 loaders Oct 25 11:18:53 it doesn't have 2 loaders, it has N loaders Oct 25 11:18:58 dmlloyd: the first loader it uses to do analysis (the temp loader) Oct 25 11:19:06 you can create an arbitrary number of temp CLs Oct 25 11:19:10 dmlloyd: the second loader is the one the application uses Oct 25 11:19:20 I don't see that in the spec anywhere Nihility Oct 25 11:19:29 not in the SPI anyway Oct 25 11:19:59 dmlloyd: its there Oct 25 11:20:18 " None of the classes loaded by this class loader will be visible to application components" Oct 25 11:20:21 (temp loader) Oct 25 11:20:40 "class redefinition that gets loaded by the loader returned by the PersistenceUnitInfo#getClassLoader method" Oct 25 11:20:57 so the problem is we dont want our dups to see the redefined classes Oct 25 11:21:06 until component install anyway Oct 25 11:21:09 smarlow: yum looks right Oct 25 11:21:13 where the application uses the classes Oct 25 11:21:39 smarlow: yup* Oct 25 11:22:00 smarlow: the first one I mean Oct 25 11:22:10 smarlow: wait, no Oct 25 11:22:13 smarlow: super(delegate) Oct 25 11:22:50 the reason is so the JPA provider can get a Class do lots of slow reflection on it Oct 25 11:22:51 dmlloyd: good eye, I'll that! Oct 25 11:22:59 pgier: ping Oct 25 11:23:04 then decide what the redefinition is going to do Oct 25 11:23:07 dmlloyd: I'll try that, thanks! Oct 25 11:23:16 smarlow: no, wait, use yours Oct 25 11:23:27 don't bother with super(delegate), that's just going to make things weird Oct 25 11:23:50 Nihility: yeah but I don't get what SPI implements this Oct 25 11:23:58 dmlloyd: i just pasted it Oct 25 11:24:12 you pasted some stuff from getNewTempClassLoader() Oct 25 11:24:19 which afaik just delegates to the deployment CL Oct 25 11:24:20 the JPA provider calls getNewTempClassloader() does scanning Oct 25 11:24:37 then later on it adds a transformer to classes already loaded Oct 25 11:24:48 on the classloader returned by getClassLoader Oct 25 11:24:58 oh, so you're saying getNewTempClassLoader() doesn't delegate to the module? it's actually a *copy* of the module? Oct 25 11:25:17 speak sense man! :) Oct 25 11:25:22 dmlloyd: the reverse Oct 25 11:25:30 the reverse of which? Oct 25 11:25:38 getNewTempClassLoader IS the module until application code runs Oct 25 11:25:44 then getClassLoader MUST be the module Oct 25 11:25:59 dmlloyd: it needs to be like a (shallow) clone of application module classloader Oct 25 11:26:06 they are tossing a Classloader and switching to a new one, the one that can create brand new classes Oct 25 11:26:14 I read it more as, every time you call gNTCL you get a new module which contains all the deployment resources Oct 25 11:26:27 but getCL is the one that the AS really uses Oct 25 11:26:50 and we can create the getCL one right away (as long as nothing uses it until the JPA provider has a chance to do it's bidness) Oct 25 11:26:59 dmlloyd: its a chicken egg problem, nothing can access a class on the module classloader until the transformer has been added which happens at a later time after classes have already been loaded Oct 25 11:27:22 but if the classes are loaded by getNTCL then we don't care as only the JPA provider sees that "copy" Oct 25 11:27:31 dmlloyd: also we probably dont want other DUPs to see the transformerd classes Oct 25 11:27:35 exactly Oct 25 11:27:46 getNTCL should create a new module, only visible to it Oct 25 11:28:05 then we should remove the module after the createContainerEntityManagerFactory() call returns as the docs say Oct 25 11:28:07 dmlloyd: right but if another DUP loads a class from getCl then we cant add a transformer its too late Oct 25 11:28:25 yeah, so we have to make sure that JPA gets the first crack Oct 25 11:29:07 looks like we can at least use our good annotation scanner to fill out PersistenceUnitInfo Oct 25 11:29:38 dmlloyd: that might be a problem with parellel deployment Oct 25 11:29:57 dmlloyd: unless we want to make everything wait on JPA Oct 25 11:30:05 logically we have to Oct 25 11:30:11 JPA currently does the createContainerEntityManagerFactory at INSTALL Oct 25 11:30:20 dmlloyd: here is the idea i was thinking of Oct 25 11:30:21 furthermore, two JPA deployments which reference each other = SNAFU Oct 25 11:30:23 which requires datasources Oct 25 11:30:47 dmlloyd: in the non-hibernate case, we could have a special modules thing that tosses and installs a new CL before component install Oct 25 11:31:08 all of the scanning uses the same module cl Oct 25 11:31:23 its only app code that needs to start a new cl with a new transformer Oct 25 11:31:40 and we only have to do this for things like toplink Oct 25 11:31:45 we can't toss the real CL once we create it Oct 25 11:32:00 otherwise anything referencing the old one will leak Oct 25 11:32:05 and get generally screwed up Oct 25 11:32:21 dmlloyd: otherwise we could redefine the module, but cl replacement sounds easier Oct 25 11:32:51 dmlloyd: i cant think of anything that would refer to it, it would just be stuff in the DUP chain Oct 25 11:32:52 I just say, if some classes get loaded before the transformer goes in, then we just say "Don't Do That" Oct 25 11:33:19 dmlloyd: unfortunately the spec does do it Oct 25 11:33:32 we can't have two CLs floating around Oct 25 11:33:38 for the same deployment Oct 25 11:33:47 getNTCL has a strict scope Oct 25 11:33:48 why not how is it different than redefining a module? Oct 25 11:34:04 redefining the module doesn't toss out the existing classes Oct 25 11:34:06 they stay around Oct 25 11:34:09 the classloader will be GCd Oct 25 11:34:17 along with all the other classes Oct 25 11:34:17 but not if things reference it Oct 25 11:34:23 because the dups will go away Oct 25 11:34:26 and if things aren't referencing it, we didn't need it in the first place Oct 25 11:34:37 if they do that though then we leak with one cl Oct 25 11:34:46 doesnt matter if we do or do not replace it Oct 25 11:35:24 I don't see how - we have the regular module that we always create, and then JPA has it's temp CLs that it creates whose scope is by spec limited to the duration of the call to createContainerEntityManagerFactory Oct 25 11:35:42 thats not entirely correct Oct 25 11:35:45 those get unloaded and removed - if the JPA impl leaks then it's limited to the JPA provider Oct 25 11:36:02 the scope of getCl is also limited to be post analysis by contrac Oct 25 11:36:15 no, getCL doesn't say anything about that Oct 25 11:36:27 dmlloyd: yes it does! Oct 25 11:36:45 "Returns ClassLoader that the provider may use to load any classes, resources, or open URLs." Oct 25 11:36:48 that's about it Oct 25 11:37:00 dmlloyd: "Add a transformer supplied by the provider that will be called for every new class definition or class redefinition that gets loaded by the loader returned by the PersistenceUnitInfo#getClassLoader method. The transformer has no effect on the result returned by the PersistenceUnitInfo#getNewTempClassLoader method. Classes are only transformed once within the same classloading scope, regardless of how many persistence units they may be Oct 25 11:37:00 part of." Oct 25 11:37:20 that's not contradictory Oct 25 11:37:27 that just says that we add the transformer to the module Oct 25 11:37:41 BEFORE LOADING ANY ENTITY CLASSES FROM IT Oct 25 11:37:52 that's up to the provider Oct 25 11:38:02 "every NEW class definition..." Oct 25 11:38:06 yep therefoore we have to account for it Oct 25 11:38:10 no, they do Oct 25 11:38:12 i mean its fucking obvious how this works Oct 25 11:38:21 i dont get why you dont understand Oct 25 11:38:32 you cant load an entity before you redefine it Oct 25 11:38:38 it's the provider's responsibility Oct 25 11:38:51 1) we call their createContainerEntityManagerFactory() Oct 25 11:38:59 no its not just about the provider Oct 25 11:39:03 its also about the application Oct 25 11:39:05 2) that method can call getNTCL() to get their stuff Oct 25 11:39:10 the class that is loaded from getClassloader Oct 25 11:39:10 3) they add transformers as needed Oct 25 11:39:15 has to be the one the provider intedned Oct 25 11:39:19 which is the transformed classes Oct 25 11:39:28 if we load the class before transforming it Oct 25 11:39:30 4) they're done, the temp CL(s) get tossed, and then getCL() gets the module with transformers on it Oct 25 11:39:37 we violate the contract between the provider and the app Oct 25 11:39:39 yeah but why would we do that? we have dependencies which stop that Oct 25 11:39:59 because we have parellel boot Oct 25 11:40:12 the DUP for JPA would be called in the modularize step when the module is created but before it's available to other deployments Oct 25 11:40:19 waiting for JPA is unnessecary and slow Oct 25 11:40:39 and we dont even need to do this for hibernate Oct 25 11:40:59 yeah we'd only call the DUP if a provider is specified Oct 25 11:41:10 but other DUPs are going to *use* these classes for stuff, like injection and linking Oct 25 11:41:16 how about a different solution, adding a module classloader flush method Oct 25 11:41:23 that won't work smarlow Oct 25 11:41:26 you can't unload classes Oct 25 11:41:43 either the CL is available or it isn't Oct 25 11:41:45 yeah, if dups keep references to them especially Oct 25 11:41:46 we also dont want our dups to see the stuff this thing makes Oct 25 11:41:56 making it available and then replacing it is just going to cause leaks and weird CCEs Oct 25 11:41:59 its going to change the definition of the class Oct 25 11:42:05 so new methods new fields etc Oct 25 11:42:44 although by luck we dont have any that i know of that look at this Oct 25 11:42:55 but i could see jpa providers adding annotaitons or something Oct 25 11:43:21 how is it going to cause a leak? Oct 25 11:43:36 the only way it leaks is if something holds a ref in the DUP chain Oct 25 11:43:47 and if its doing that its going to leak the deployment CL anyway Oct 25 11:43:53 the only thing that dependency modules are used for is linking Oct 25 11:43:54 afaik Oct 25 11:44:41 I would be surprised if we don't have at least one DUP that keeps references to something it saw Oct 25 11:44:51 then we have a massive leak right now Oct 25 11:45:04 the spec doesn't say anything about the scope of the getClassLoader CL, but I get the implication that they mean the actual deployment CL Oct 25 11:45:07 the DUP chain cant hold references PERIOD Oct 25 11:45:13 I'm looking to see if there's something that clarifies Oct 25 11:45:26 the DUP chain doesn't hold references, but the deployments they process do Oct 25 11:45:54 a wierd CCE i could see Oct 25 11:46:08 if one dup held a class definition Oct 25 11:46:13 and another later one got a surprise Oct 25 11:46:47 "In a Java EE environment, the classes of the persistence unit should not be loaded by the application Oct 25 11:46:47 class loader or any of its parent class loaders until after the entity manager factory for the persistence Oct 25 11:46:47 unit has been created." Oct 25 11:46:55 seems to support what I'm saying... Oct 25 11:46:57 still looking though Oct 25 11:48:09 it says that getClassLoader is the cl the application code sees Oct 25 11:48:12 if it is, then it's on us to stop other classes from being loaded from a PU module before the entity manager factory boots Oct 25 11:48:26 it also says the application sees the transformed classes Oct 25 11:48:38 and the provider is allowed to transform any class in there it wants to Oct 25 11:48:39 okay, so it seems pretty clear then Oct 25 11:48:51 in order for that to logically happen Oct 25 11:49:04 provider can transform whatever it wants, and nobody's allowed to use the CL until it does so Oct 25 11:49:05 that classloader cant load anything that cant yet be transformerd Oct 25 11:49:15 right, so like the spec says Oct 25 11:49:24 no loading classes until the EMF boots Oct 25 11:49:41 otoh, if there is no custom provider, then we skip that DUP and everyone's happy Oct 25 11:49:48 yeah and creation of EMF is a late phase thing Oct 25 11:50:07 late but can still be first in the post-CL series Oct 25 11:50:09 imagine if two specs did this Oct 25 11:50:17 yeah, it wouldn't work Oct 25 11:50:30 and, this is also fragile in that two interdependent DUs are by spec invalid Oct 25 11:50:31 only way it works with 2 is tossing the cl Oct 25 11:50:39 only way it works with parellel booting is tossing the cl Oct 25 11:50:46 I don't see why you think so Oct 25 11:50:52 our DU is broken into phases Oct 25 11:51:03 parallel boot is done by interdependent phases Oct 25 11:51:28 yeah so you are either going to introduce a new phase Oct 25 11:51:30 the spec forbids loading app classes from a persistence unit until after the EM boots Oct 25 11:51:34 which can only have one dup in it Oct 25 11:51:48 nah just tag the EM boot at the end of modularize after the module is created Oct 25 11:51:49 that'd do it Oct 25 11:52:03 then when post-module runs, the EM is already booted Oct 25 11:52:06 EM boot is component install time frame Oct 25 11:52:21 sure, but that'd clearly have to change for this to work Oct 25 11:52:26 either way everything has to stop and wait for JPA Oct 25 11:52:35 and if another spec does this its impossible Oct 25 11:52:43 circular blocking Oct 25 11:52:47 it's in the spec Oct 25 11:52:55 well, changing the EM start phase, turns other dependencies also, like datasources Oct 25 11:53:13 does the EM require datasources to boot? Oct 25 11:53:16 yes Oct 25 11:53:28 datasources are pre-classloading aren't they? Oct 25 11:54:15 annotation scanning and descriptor processing are pre-classloading as well Oct 25 11:54:51 note that the spec requirement about loading classes from the classloader seems to be referring to the app behavior Oct 25 11:55:05 what else would load classes? Oct 25 11:55:15 the container itself Oct 25 11:55:54 i guess as long as the dup is only added for non-hibernate im fine Oct 25 11:56:19 it just seems like recycling the cl is more flexible and much faster Oct 25 11:56:43 but maybe we want hibernate to be faster! :) Oct 25 11:56:55 maybe it is, I'm just not clear on what that solves, because either way the deployment is contingent on the entity manager Oct 25 11:57:34 and the old CL will certainly stay around if other deployments have a chance at linking to it (which they will the moment it hits POST_MODULE) Oct 25 11:57:53 any classes loaded at that point are there forever, and if you redefine them you'll have CCE hell Oct 25 11:58:00 sure just like they can link to the temp cl Oct 25 11:58:10 anything can leak i think its a red herring Oct 25 11:58:23 yeah but then you can't pass the entity classes to and from the PU module Oct 25 11:58:28 yo'll get CCEs Oct 25 11:58:53 if I have an EJB jar and a PU jar and the PU jar gets a module right off, and the EJB jar links against the untransformed PU classes Oct 25 11:59:09 then when the PU gets transformed and you try to read them from the EJB JAR, you'll get CCE Oct 25 11:59:34 because the EM will be using transformed classes from the new CL, but the EJB will be static linked against the old CL Oct 25 11:59:40 so you mean if someone constructs an instance of something in a linked module Oct 25 11:59:54 yeah, which is really the only reason you'd *need* the temporary CL in the first place Oct 25 12:00:05 i.e. to load a class from the module before the EM boots, against spec Oct 25 12:00:46 * dmlloyd notes that he's waaay too lazy to read specs unless he gets into an argument about them :) Oct 25 12:01:37 well i still argue the EM spec is talking about the app perspective, but yeah you're right we have to toss all refering CLs as well since that could happen which is back to the nastiness of redefining all modules Oct 25 12:02:02 that's mainly what I'm worried about - once you change the CL after post-module everything goes nuts Oct 25 12:02:22 change the CL, comma, ... Oct 25 12:02:39 honestly im just pissed it does this double classloading nonsense Oct 25 12:02:46 if it really wants to do transfomation Oct 25 12:02:50 it shouldnt load the fucking class Oct 25 12:02:51 :) Oct 25 12:03:35 smarlow: what phase is EMF completed in right now? Oct 25 12:04:12 it seems the only possible spot is immediately before post-module Oct 25 12:04:16 Nihility: INSTALL phase Oct 25 12:04:25 (after creating a dummy module def) Oct 25 12:05:08 i believe drivers are also install Oct 25 12:05:39 the jndi binding wont happen until install Oct 25 12:06:15 but the emf would still wait for them Oct 25 12:06:17 so that should be ok Oct 25 12:06:30 the jpa persistence provider needs the jndi lookup of the datasource to work Oct 25 12:06:58 smarlow: before install completes? Oct 25 12:06:58 so that means, the emf would wait until install time? Oct 25 12:07:10 the JNDI lookups should be in place by then Oct 25 12:07:28 that info comes from jandex and descriptors Oct 25 12:07:49 the question we need to answer Oct 25 12:08:00 is does JPA need to run queries Oct 25 12:08:06 or use the jdbc connection Oct 25 12:08:08 BEFORE Oct 25 12:08:22 all transformation has been done Oct 25 12:08:27 yeah maybe it's not even an issue Oct 25 12:08:28 and getClassLoader can be used to load classes Oct 25 12:08:45 otoh though there's probably a ton of crap in INSTALL that could be earlier Oct 25 12:08:58 if it does need the connection we are screwed Oct 25 12:09:41 I'm thinking that they wouldn't use the datasource to lookup a connection at EMF creation time, because we wouldn't create an actual EM yet. But, checking for clarification Oct 25 12:09:43