Community
Participate
Working Groups
This looks like a regression, worked for me in last week's integration builds. How to reproduce: 1. create new Hello World ANSI C project, confirm perspective switch to C/C++ 2. build project 3. set a breakpoint on line with 'puts' 3. Debug As > Local C/C++ Application Perspective switch confirmation dialog should appear, but doesn't. Eclipse Platform 2021-12 (4.22), I20211012-1800 org.eclipse.cdt 10.5.0.202110041817
Hi Martin, thanks for the bug report and testing with prerelease versions. Have you tried the switching in Java version and/or a new workspace? I would like to know if this is a CDT issue, or if the bug needs to be passed to platform debug team. Thanks!
Hi, I tested it with clean workspace, Java project works as expected - perspective switch confirmation dialog is shown.
Thanks, I'll try to reproduce it so I can track down this regression.
I have reproduced this problem. Thanks for the report. CDT debug uses a lot of adapters, so that is my first guess as to what change may be an issue here - Bug 576024. FWIW the issue is not switching perspective, but the debug session not fully starting if the debug session is run before the Debug View is open. i.e. if you open the Debug view (instead of switching to Debug perspective) the debug session continues its startup normally and then the switch prompt comes up.
I have verified that the issue is down to Bug 576024 - i.e if I checkout rt.equinox.bundles 053204eb5c5bc0a9f7f834b17546413e76a20b82 things work fine. Next step, trace which call to get adapter (or similar) changes unexpectedly.
It looks like the AdapterManager change means a different class is being returned by getAdapter: Before: get adapter on org.eclipse.cdt.dsf.gdb.launching.GdbLaunch for interface org.eclipse.debug.ui.contexts.ISuspendTrigger would return org.eclipse.cdt.dsf.gdb.internal.ui.GdbSuspendTrigger. it now returns org.eclipse.debug.internal.ui.contexts.LaunchSuspendTrigger Next step - why? But my guess is that it is related to loaded vs not-loaded as this only affects first launch.
(In reply to Jonah Graham from comment #6) > Next step - why? But my guess is that it is related to loaded vs not-loaded > as this only affects first launch. That's quite possible. The change for adapters implements an heuristic that prefers adapters from already loaded bundles (to prevent from activating more bundles than necessary). I think this heuristic was not in place previously; so it can affect such case. Note that the case itself is not really correct and relies on unspecified behavior (ie adapter order); it could also have been causing trouble to some other plugins where the default LaunchSuspendTrigger would work better than the GdbSuspendTrigger; or have caused CDT to be activated even when doing some unrelated work because the adapter got invoked on unrelated bits. I'm not sure that the default LaunchSuspendTrigger adapter is meant to be overridable in 1st place. One test/workaround you may try is to programmatically activate the bundle when it's present.
The issue is with the heuristic with force loading. The case that is the problem is when CDT is trying to force load an adapter and with the change in Bug 576024 which adapter is selected changes. CDT is force loading (i.e. calling IAdapterManager.loadAdapter) and has a declared adapter factory for that type. In this case Eclipse should not choose a less specific adapter factory to avoid loading. Follow up Comment 6: - GdbSuspendTrigger adapted GdbAdapterFactory from GdbLaunch - GdbLaunch is a subclass of Launch So using the javadoc of AdapterManager, the search order should be to use the target class (GdbLaunch) first. I have a patch and I am working on a testcase now.
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/186491
The patch for Bug 576024 does something I don't think it should have done. The original idea was to have multiple adapter factories that could adapt the same type to the same output. e.g. have two different factories that can be registered to adapt ExtensionBasedTextEditor -> IContentOutlinePage and use additional information available at runtime to control whether they can actually do the adapting (such as based on the language being displayed in the editor). The patch that is merged now does achieve that goal, however it also change the behaviour based on loading, of pairs that don't conflict. For example, if you have an adapter factory registered for ExtensionBasedTextEditor -> IContentOutlinePage and another factory for MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage then requesting adaption from MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage should never use the factory of ExtensionBasedTextEditor -> IContentOutlinePage regardless of activation states. It is this second part of the change that causes CDT to be broken. Therefore I think the exit case for looping through available factories has to stop if the actual class being requested is queried.
(In reply to Jonah Graham from comment #10) > For example, if you have an adapter factory registered for > ExtensionBasedTextEditor -> IContentOutlinePage and another factory for > MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage then requesting > adaption from MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage > should never use the factory of ExtensionBasedTextEditor -> > IContentOutlinePage regardless of activation states. That's indeed an issue and this behavior is specified in AdapterManager clearly enough. Unfortunately, this was not detected by a test. We should consider adding some tests dedicated to ensure we pick the "most spcific" adapters as specified.
(In reply to Mickael Istria from comment #11) > (In reply to Jonah Graham from comment #10) > > For example, if you have an adapter factory registered for > > ExtensionBasedTextEditor -> IContentOutlinePage and another factory for > > MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage then requesting > > adaption from MySubclassOfExtensionBasedTextEditor -> IContentOutlinePage > > should never use the factory of ExtensionBasedTextEditor -> > > IContentOutlinePage regardless of activation states. > > That's indeed an issue and this behavior is specified in AdapterManager > clearly enough. Unfortunately, this was not detected by a test. We should > consider adding some tests dedicated to ensure we pick the "most spcific" > adapters as specified. I am pleased that I explained it well enough that we are on the same page :-) I will have a go writing a test. The complication is that we either have to add a new plug-in that isn't activated, or implement a test version of IAdapterFactoryExt that we can mock the lifecycle with. I am leaning towards the latter but would be grateful if you knew one way or the other which was best.
(In reply to Jonah Graham from comment #12) > [...] but would be grateful if you knew one way or the other which was > best. Sorry - there are comments on this topic in the gerrit. Catching up now.
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/186540
Gerrit change https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/186491 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=d4f22041029e891c293bd778a04fa966e7bc04a0
Last I-Build includes the proposed fix. @Jonah can you please verify it?
Gerrit change https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/186540 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=7430bbb1a7a29ea54c27c673f66f69e1808da245
Works for me, tested on: Eclipse Platform I20211016-1800 org.eclipse.cdt 10.5.0.202110041817 org.eclipse.equinox.common 3.15.100.v20211015-1726
Thanks a lot Martin for the report; Jonah for the analysis and fix, and Christoph and Andrey for the reviews and further improvements!
(In reply to Mickael Istria from comment #16) > Last I-Build includes the proposed fix. > @Jonah can you please verify it? Verified on I20211017-1800
