Bugzilla – Full Text Bug Listing
| Summary: | Viewing an OSGi state dump fails | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Chris Frost <eclipse> | ||||
| Component: | Components | Assignee: | Glyn Normington <glyn.normington> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | glyn.normington, tjwatson | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 362730 | ||||||
| Attachments: |
|
||||||
|
Description
Chris Frost
Created attachment 205904 [details]
The dump created
The NPE is caused because an attempt is made to determine the region of a bundle id using the reconstituted region digraph and the bundle id is not known to the region digraph (or at least not to its mapping of bundle id to region). On further inspection, the reconstituted region digraph appears to be corrupted. The mapping of bundle id to region in the main digraph class shows multiple bundle ids associated with a coregion, but the coregion does not exist. Some bad smells have crept into the region digraph code which may perhaps account for this. The main digraph class shares some of its internal state with BundleIdBasedRegion. The state shared is the bundle id to region mapping and the monitor of the main digraph class, which is used to serialise access to the mapping. This complicates the synchronisation protocol of the main digraph class and could potentially have introduced thread safety issues. The code should be inspected to ensure that the bundle id to region mapping is kept consistent with the collection of extant regions. I am leaving this bug against Virgo until I have had a chance to analyse the region digraph code further to ascertain whether it contains the bug. (In reply to comment #2) > Some bad smells have crept into the region digraph code which may perhaps > account for this. I think the odor you are smelling could come from the changes that went into the fix for bug 342979. In particular see bug 342979 comment 4 The following commits are related to this. http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=be247703a60efd3484425219e497ad9fe2b05776 http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=dfd6a8a17a0c8885797be2d6c0e786fd27927d14 (In reply to comment #3) Thanks for reminding me of the specific bug and commits. I certainly feel responsible for the bad smell as I agreed to the changes. I need to re-read the code to see if there is a hole that is causing this bug and if there might perhaps be a cleaner approach. There seems to be a bug in StandardRegionDigraph.removeRegion which does not update the bundleToRegion map. A region can be removed which still contains bundles, although this probably only makes sense in the "offline" side-state resolution scenario which is involved here (a temporary "coregion" is used to contain bundle ids of speculative bundles which may or may not be committed to the framework later, depending on the outcome of resolution). (In reply to comment #5) > There seems to be a bug in StandardRegionDigraph.removeRegion which does not > update the bundleToRegion map. A region can be removed which still contains > bundles, although this probably only makes sense in the "offline" side-state > resolution scenario which is involved here (a temporary "coregion" is used to > contain bundle ids of speculative bundles which may or may not be committed to > the framework later, depending on the outcome of resolution). A new test should be added to catch this bug before it is fixed. I am still to analyse the code structure too to see if can be improved. I suspect it can, e.g. by factoring out helper methods to maintain the bundleToRegion map in BundleIdBasedRegion and then moving these methods to StandardRegionDigraph and possibly exposing them via a new interface implemented by StandardRegionDigraph. Thread safety improvements should be considered at the same time. Glyn, can this bug be moved to Equinox->Components and closed since you released the fixes to equinox master ending with the following commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=d744d90b5565ac676fcccf3d5fdd35b7f057b3f4 Or did you have more work to do in Virgo for this. If so perhaps a separate Virgo bug is in order? Done. I've spawned bug 362730 for the sequel in Virgo. |