Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 342453

Summary:

[launch] Launch configuration listeners called in unexpected way

Product:

[Eclipse Project] Platform

Reporter:

John Cortell <john.cortell>

Component:

Debug

Assignee:

Platform-Debug-Inbox <platform-debug-inbox>

Status:

CLOSED WONTFIX

QA Contact:

Severity:

normal

Priority:

CC:

Michael_Rennie, pawel.1.piech

Version:

4.1

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

stalebug

Attachments:

Description	Flags
Sample listener plugin (project)	none

Description John Cortell

2011-04-11 11:28:36 EDT

ILaunchConfigurationListener.launchConfigurationChanged() is called with a new launch configuration instance every time there's a configuration change. This is not what a client would expect. The listener interfact has a method for being notified when a config is created, and another when a config is removed. The third method rounds out the set with a method for being notified when the config changes. Seems pretty straight forward. But here's what it looks like in practice; the following is output from a simple plugin that registers a listener with the launch manager then output the System.identityHashCode(o) of the config received in the notification. These listener ignores all notifications for working copy configs.

   config added: 27324458     // created the config
   config changed: 543626     // make a change
   config changed: 24142117   // make another change
   config removed: 27324458   // delete the config

The addition and removal goes as expected. However, each change notification has a surprising config: not the one created (or removed), and there was never any addition notification for those configs.

Comment 1 John Cortell

2011-04-11 11:33:13 EDT

Created attachment 192940 [details]
Sample listener plugin (project)

Comment 2 Michael Rennie

2011-04-11 14:47:09 EDT

(In reply to comment #0)
 
> The addition and removal goes as expected. However, each change notification
> has a surprising config: not the one created (or removed), and there was never
> any addition notification for those configs.

That would be due to this code:

// notify of add/change for both local and shared configurations - see bug 288368 
if (added) {
  getLaunchManager().launchConfigurationAdded(new LaunchConfiguration(getName(), getContainer()));
} else {
  getLaunchManager().launchConfigurationChanged(new LaunchConfiguration(getName(), getContainer()));
}

from LaunchConfigurationWorkingCopy#writeNewFile(..).

For the life of me I can't recall why we are making new instances of LaunchConfiguration for the notification, rather than simply calling the notifier with 'this' like:

if (added) {
  getLaunchManager().launchConfigurationAdded(this);
} else {
  getLaunchManager().launchConfigurationChanged(this);
}

at this stage, since the file has been saved and the parent value set we could also always return the original LaunchConfiguration instead of a working copy, like:

if (added) {
  getLaunchManager().launchConfigurationAdded(getOriginal());
} else {
  getLaunchManager().launchConfigurationChanged(getOriginal());
}

Comment 3 Michael Rennie

2011-04-11 14:47:52 EDT

For reference bug 288368 introduced the code for always creating new LaunchConfiguration instances for the notifier.

Comment 4 John Cortell

2011-04-11 14:52:03 EDT

Looking at the patch for bug 288368, it seems the creation of a new launch configuration for a *change* notification was already there.

Comment 5 Lars Vogel

2019-11-14 03:44:27 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

If the bug is still relevant, please remove the "stalebug" whiteboard tag.