[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15890395#comment-15890395 ] Karl Pauls commented on FELIX-5573: --- Ok, I can confirm that we do return null in this case. I created a test case that reproduces the issue. Luckily, the immediate fix seems easy. Basically, all we are missing is a check in BundleClassloader.loadClass for the class to be null and throw a CNFE in that case. This is consistent with both, bundle.loadClass and (more importantly) Classloader.loadClass API (I don't think any of the two really allow to return a null). Regarding FELIX-5570, I think it will now work as [~djencks] describes it above i.e., the attempt to get the WeavingHook from the service factory will cause a CNFE inside the service factory which will make it return null (or maybe an exception) which in turn, makes us ignore the weaving hook for this classload. It sounds like this should get us on par with equinox for the moment. Granted, I think [~tjwatson] raised another question in FELIX-5570 namely, whether it would be possible to detect that the cycle was due to this special situation and resolve it without throwing any exception but I think that discussion can continue somewhere else. I'm going to rename this issue to reflect that it is about not return null in a cycle but rather throw an CNFE. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889820#comment-15889820 ] Karl Pauls commented on FELIX-5573: --- Right, throwing some kind of CNFE might be one solution to it but first things first - right now, we don't expect to return null in the first place. This is a special case that makes that happen but we "explicitly" assume it will not. The relevant comment is: // If a cycle is detected, we should return null to break the // cycle. This should only ever be return to internal class // loading code and not to the actual instigator of the class load. It sounds like this assumption doesn't hold in the case of a class load of a class that requires to be woven "by itself". In other words, we use null as a result to signal to _internal_ code that this was a cycle and needs to be dealt with accordingly. In this case our _internal_ code doesn't deal with it (or so it appears) and if that is true it needs to be addressed so that it does. That is when the question of how comes up. Whether the fix is to throw an exception or to ignore the cycle in this case and allow the class load to succeed unwoven we have to see. Regardless, it should be because we understand that this is going on and not an accident. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889790#comment-15889790 ] Felix Meschberger commented on FELIX-5573: -- My understanding is like David's: I think Bundle.loadClass should not return null as this sounds unexpected, although the specification is not entirely clear. So I would sort this under the topic "managing expectations". How about throwing a new "ClassLoadingCircularityDetectedException extends ClassNotFoundException" ? This way callers get the "ClassNotFoundException" while internally you can differentiate between the cases. Plus logging the exception will properly indicate the actual problem: the circularity. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889751#comment-15889751 ] Karl Pauls commented on FELIX-5573: --- I agree, it might turn out to not be a bug but I want to investigate the situation as we clearly didn't expect that it would happen due to outside conditions. I might be wrong but as far as I understood the situation we are talking about two different null values - the one for getService and the one for getting the class. I'm interested in the latter - hence, this issue which may or may not be relevant for FELIX-5570. It so happens that it is triggered by the use-case in there but it might need to be (or not to be) addressed regardless. I'll try to look into it soon and report back here. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889096#comment-15889096 ] David Jencks commented on FELIX-5573: - I don't see why there's a problem returning null if there's some problem such as recursion in class loading. My question was what is supposed to happen if getService for one of the weaving hooks returns null. If this is supposed to prevent the original class load then weaving hooks can't be DS components. If that hook is supposed to be ignored (presumably with logging) then the original class load ought to succeed and DS succeed in creating the weaving hook. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889029#comment-15889029 ] Richard S. Hall commented on FELIX-5573: Rightly or wrongly, we explicitly chose to return null in this case because we were under the assumption that this situation would only occur for internal framework operation (i.e., we didn't expect null to be returned to end user code). We explicitly check for null being returned and know we can ignore that case when we detect a cycle. So, if we just start throwing a CNFE, then we might have some difficulty ignoring it or knowing when to ignore it in the framework. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (FELIX-5573) Detect recursive class loads while invoking weaving hooks
[ https://issues.apache.org/jira/browse/FELIX-5573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889008#comment-15889008 ] Karl Pauls commented on FELIX-5573: --- One example of this is FELIX-5570 where we apparently just return null (we should probably make sure we throw a CNFE if we detect a recursive class load that doesn't involve weaving hooks too) which makes it so that SCR blows up. > Detect recursive class loads while invoking weaving hooks > - > > Key: FELIX-5573 > URL: https://issues.apache.org/jira/browse/FELIX-5573 > Project: Felix > Issue Type: Bug > Components: Framework >Affects Versions: framework-5.6.2 >Reporter: Karl Pauls >Assignee: Karl Pauls > Fix For: framework-5.6.4 > > > We need to detect recursive class loads while invoking weaving hooks, if > recursion is detected for a class name then we want to simply ignore all > weave hooks for the recursive class load and allow it to proceed to define > class. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
