Bugzilla – Full Text Bug Listing
| Summary: | Don't invoke the engine when loading/saving profile preferences | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Pascal Rapicault <pascal> | ||||||||||
| Component: | p2 | Assignee: | John Arthorne <john.arthorne> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | critical | ||||||||||||
| Priority: | P1 | CC: | caniszczyk, cocoakevin, dj.houghton, irbull, kim.moir, Mike_Wilson, Olivier_Thomann, pwebster, simon_kaegi, tjwatson | ||||||||||
| Version: | 3.5 | ||||||||||||
| Target Milestone: | 3.5 M6 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Pascal Rapicault
Is it the build itself that's busted, or simply updating from the 3.5 I build p2 repo? PW FWIW I was able to come up after the update however my config.ini had the mega osgi.bundles list and simpleconfigurator was not on that list at all. Looking at my new profile both simpleconfigurator and it's CU were present and there were the appropriate actions in the CU. All I can think of is that simpleconfigurator didn't bind to any fragment so was installed according to the engine but no actions were run. Only the update from the I20090224 is not working. The SDK download is fine but will also have a similar problem updating to next week I build which is why we are looking at a fix for this week. Created attachment 127379 [details]
new garbage collector bundle
As Pascal mentioned, this is a problem with last week's i-build and the problem still exists in this week's i-build. The basic scenario is:
- you select a new version of the SDK to update to
- we download all the JARs
- we uninstall the old JARs
- the profile preferences get triggered and loaded
- this kicks the engine and starts the GC
- the GC notices that things have been uninstalled so it wants to run
- it removes all the new bundles you just downloaded because they haven't been installed yet (and are not referenced by the profile)
The work around for now is to disable the p2 garbage collector. The attached JAR disables it by default although you can set a system property (equinox.p2.gc.enabled) if you really want it to run. Note that the GC application (as an Eclipse app) should still run ok no matter what the value of the system property is. Alternatively you can disable the GC via preferences.
This interaction between garbage collection and the profile preferences caught us by surprise, but it revealed a number of areas where we can make improvements to avoid this kind of problem: - We have learned from this bug and others that recursive invocations of the engine can have unexpected results, because much of our code assumes only one engine session per profile at any given time. We will change the engine locking to disallow re-entrant calls to the engine on the same profile (bug 266388). - The GC trigger policy is too simplistic, as it just checks that there was a commit event after an uninstall event, without checking that the two events occurred against the same profile. Bug 266941 has been entered to improve this trigger policy to avoid triggering GC at the wrong time. - When bundles are deleted out from under us during an install, the install completes successfully but leaves the system in a broken state. An exception should have been triggered and the engine session should be rolled back (bug 266925). I will take this bug to eliminate the recursive invocation of the engine when loading/saving profile preferences. This change was underway already, but this bug highlights the urgency of doing it immediately. Created attachment 127416 [details]
Fix v01
Created attachment 127419 [details]
Test case
This test case performs the exact scenario that caused the upgrade to fail. It performs an upgrade in which one of the upgraded bundles adds a repository via a touchpoint action, and asserts that all the expected artifacts are in place when the test completes. I verified without the fix in place, this test case fails because the GC deletes artifact for the bundle being installed. With the fix the artifact is left intact and the upgrade succeeds.
Created attachment 127420 [details]
Fix v02
Simon, please double-check this new patch. I had to remove the check from SimpleProfileRegistry.getProfileDataDirectory which had an assertion to ensure it occurred inside an engine operation. We now need to access this directory from the profile preferences, so I had to remove this check. There is only one other caller of this method (inside the engine), so I don't think the assertion was buying us much.
Yep. It's ok to remove the check in getProfileDataDirectory. I took a look over your previous patch and thought it looked fine and must say it looked smaller too. I can take a better look tomorrow however if you're ready then commit. Thanks Simon. Fix and test released to HEAD. *** Bug 267047 has been marked as a duplicate of this bug. *** *** Bug 267134 has been marked as a duplicate of this bug. *** |