| Summary: | [osgi] Application update through update site causes OSGI error on restart | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Todd E. Williams <todd> | ||||
| Component: | Runtime | Assignee: | Thomas Watson <tjwatson> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | birsan, jeffmcaffer, tjwatson | ||||
| Version: | 3.0 | ||||||
| Target Milestone: | 3.0.1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Todd E. Williams
The multiple prompts to the user to accept/deny perspective resets is likely caused by the old plugins being uninstalled and the newer version being installed. This can be solved with a yes/no/always type of dialog, etc. As far as I can tell, this does not appear to be an update bug, but rather a side effect of updating manifested in the UI. I'm not concerned about the dialogs asking to reset the perspective. What bothers me is that after all of that, most of the actions are disabled, as shown in the log. Basically, the software is broken without a -clean restart. Moving to runtime for investigation (as per .log's entry: Caused by: org.osgi.framework.BundleException: Internal Error in the OSGi framework. Please report this problem.) This should be investigated for 3.0.1. CC'ing Tom Do you have to use the -clean option. When I restart eclipse a second time without the -clean option it looks like everything comes up fine. I'm tracking down a bug in PackageAdmin.refreshPackages() now... Created attachment 14198 [details]
Proposed fix to PackageAdmin
There is a bug in PackageAdmin when a fragment gets uninstalled. In this case
the host bundle's classloader was getting pinned for removal pending which is
needed because a fragment got uninstalled so the content of the host changed.
The bug is in the processing of the graph of dependent bundles to refresh.
When processing the removal pending bundles the depenedents for the removal
pendings are not always added to the refresh graph. In this case the host was
jface.text because the myeclipse product has a fragment to jface.text. Since
the dependents of jface.text were not added to the refresh graph this caused
refreshPackages to fail when it tried to remove the classloader of the old
jface.text host bundle.
Thomas, -clean is exactly the issue. You *have* to use the -clean option on the restart. However, telling users this is exceedingly problematic as they immediately think the update broke their installation and become frantic. I truly believe that when the configuration is changed, the restart should *always* be a clean restart to avoid this and any future problems. Logically, the configuration has been changed, so the cached configuration is trully invalid, so the cached configuration should be dumped. OK, I understand now. I thought you meant you had to run with the -clean option when you launched eclipse again after the original errors occurred to get it to work. I had found that after the errors occurred I could simple relaunch eclipse again (without the -clean option) and everything would be fine. Relauncher eclipse with -clean everytime update performs an install/update/uninstall would cause some other problems. First of all, it would prohibit us from supporting installs/updates/uninstalls without restarting eclipse (which is a goal we want to accomplish but did not fully make it for 3.0). Also, it would prohibit any other management agent besides update from installing and managing other bundles installed in the Framework. The -clean option causes all bundles to be uninstalled and then update installs all the plugins on restart, but any other bundles that were installed by another management agent will have been lost. The reason the problem outlined in this report occurred is because of a bug in PackageAdmin. We should fix the bug in PackageAdmin and keep moving forward in making eclipse updatable without restarts. I attached a patch that fixes this bug and tested out with your scenario and everything works now. Thomas, that was a very well articulated explaination. Thanks for the quick turnaround on the fix and for making it available for 3.0.1. Well I have not actually made it available for 3.0.1 yet ;-) Need to get a code review. Jeff, could you review the patch and get it approved to include in 3.0.1? changes look good to me. looks good to me too. At the very beginning of this method, in the refresh==null branch why don't we have the same code pattern? Looks like this code could be cleaned up a bit. We are doing some additional work in the beginning of this method that will be taken care of later on when we complete the graph in the do while loop. When refresh == null we really just want to prime the gragh with the removal pending bundles, the else statements really are not even needed because fragments are take care of in the do while loop later. We should consider cleaning this up for 3.1. The patch has been checked into 3.0.1 and HEAD. See bug 72877 to track additional clean up of this code for 3.1 release. Thomas, Please mark as fixed before 3.0.1 RC1. The fix will be included in 3.0.1 RC1. |