Summary: | [DS] Component deactivation in incorrect order | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Equinox | Reporter: | raymond.cuenen | ||||||||||||
Component: | Compendium | Assignee: | Stoyan Boshev <s.boshev> | ||||||||||||
Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
Severity: | critical | ||||||||||||||
Priority: | P3 | CC: | pvanderlei, raymond.cuenen, s.boshev, sja.eclipse, tjwatson | ||||||||||||
Version: | unspecified | Keywords: | greatbug | ||||||||||||
Target Milestone: | 3.6 M7 | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Whiteboard: | |||||||||||||||
Attachments: |
|
Description
raymond.cuenen
2010-04-16 07:59:11 EDT
Created attachment 165083 [details]
Example component configuration describing the problem
Created attachment 165084 [details]
Console debug output
Created attachment 165085 [details]
Patch proposal
Thanks for the detailed bug report! Marking for greatbug. Stoyan, can you take a look? I'm unsure if this is related to bug 255279, but its Summary field is certainly similar. Let's make sure that both test cases continue to behave as expected. Didn't see bug 255279. It seems a similar situation as I describe here. I have tetsted my examble without component E (and then deactivating component D), which gives me the same, incorrect, deactivation order. I haven't tested the case of also removing component A (and making component B immediate=true), which is the situation in bug 255279 I believe. I have tested other dependency chains that work fine in both versions, original and patched. Let me know if I should attach them here also. I have downscaled my example to three components B, C, D (as described in my previous comment). This is different from bug 255279, since it still shows the same problem. I also took the liberty of testing the code of bug 255279. Running that code gives the same output for both the original and patched version. I've also attached my eclipse project that I used to test with, although it only contains this component description of the downscaled example and not the full range of component descriptions that I had. Created attachment 165108 [details]
DSTest eclipse project
Or maybe a better patch approach would be to change line 272 "scpEnabled.addElement(scp);" to "scpEnabled.insertElementAt(scp, 0);", so that the scpEnabled Vector always contains the correct order, instead of adding "Collections.revers(result);" at line 564. Still I have seen in my runtime container that the deactivation order varies from time to time. I cannot locate the exact position of the problem (yet), but sometimes the order is correct (for the patched version) and sometimes it is not. I don't know the design ideas of the Resolver class (or any of DS) but I suspect that the component dependency tree resolving algorithm doesn't fully comply with the graph theory needed for tree-traversal when deactivating service components. Anayway, to Stoyan, can you please check if any of this is valid, since simply changing the order of the "scpEnabled" Vector isn't sufficient to fully solve this problem. Ok. I came up with another solution; In my analysis it seems that the activation order is correct, so I added another Vector to the Resolver class called "scpActivated" from which the clone is used in the selectNewelyUnsatisfied method (line 457) instead of the clone of the "scpEnabled" Vector. This new "scpActivated" Vector is modified in the InstanceProcess class. In the buildComponent method I added, after line 572: if (!InstanceProcess.resolver.scpActivated.contains(scp)) { InstanceProcess.resolver.scpActivated.addElement(scp); } And in the disposeInstances method I added after line 335 and line 357: InstanceProcess.resolver.scpActivated.removeElement(scp); This solution works fine in our, fairly complex, runtime container. (In reply to comment #10) > Still I have seen in my runtime container that the deactivation order varies > from time to time. I cannot locate the exact position of the problem (yet), but > sometimes the order is correct (for the patched version) and sometimes it is > not. Yes, the proposed patch will not work because the order of the components in "scpEnabled" Vector depends on the sequence of processing the the xml files of the bundle that hold them. And later it may also depend on the enable/disable calls to the DS API. (In reply to comment #11) > Ok. I came up with another solution; I agree this solution might work in most of the cases. I think there will be problems if there are multiple threads registering services thus leading to activation of components through different threads and therefore the order of the activated components in the new vector will not be correct. I think the correct solution would be to fix the Resolver's selectNewlyUnsatisfied() logic to find the components that depend on the service that is being unregistered and they have to be deactivated because of that. Currently it finds all components that must be deactivated. This brings in the result components that do not depend on the current service being unregistered. I will try this solution and provide a patch tomorrow. Created attachment 165595 [details] proposed patch This patch implements the solution as described in comment#12 Raymond, can you please test it in your environment? (In reply to comment #13) > Created an attachment (id=165595) [details] > proposed patch > This patch implements the solution as described in comment#12 > Raymond, can you please test it in your environment? I have tested the patch in our osgi environment and I ran the tests I constructed for reporting this bug. All tests succeeded and I haven't observered any other unexpected behaviour. Thanks. Fixed in HEAD |