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

Bug 246270

Summary: events should not be fired when connection profiles are loaded
Product: [Tools] Data Tools Reporter: Karen Butzke <karenfbutzke>
Component: Connection Mgt FrameworkAssignee: Brian Fitzpatrick <bfitzpat>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: bfitzpat, clin, jpitman, lchan, ledunnel, sholars
Version: 1.6.1Flags: jgraham: pmc_approved+
sholars: pmc_approved+
lchan: pmc_approved+
bfitzpat: pmc_approved+
Target Milestone: 1.6.1RC3   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
proposed patch - HEAD stream none

Description Karen Butzke CLA 2008-09-04 15:38:44 EDT 
bug 241713 involved a deadlock in DTP because of events being fired when profiles are loaded.  This deadlock is fixed, but the underlying problem still exists and can cause problems.  Events should not be fired in this case.  If a client is calling InternalProfileManager.getProfiles(boolean) for the first time, they do not know whether the profiles are already loaded or whether they will be loaded at that time.  This leads to unexpected behavior, they could potentially get all the profiles and then also be notified that those same profiles were just added. I don't understand how it is helpful to fire change events during initialization. What if the client is asking for the profiles in order to add a listener to them?  They would end up adding duplicate listeners because of the change event being fired.

Just an aside, part of the reason the fix for bug 241713 was accepted was because it stated that the risk was low and the behavior was not changed.  This is *not* true, the behavior was changed because now events are fired for all profiles not just the "default profiles".

What are your thoughts on changing this behavior?
Comment 1 Brian Fitzpatrick CLA 2008-09-05 11:03:26 EDT 
Ouch. Ok, let's take a look at this and get back to you.

Larry, do you have any ideas on this one?
Comment 2 Larry Dunnell CLA 2008-09-08 15:26:05 EDT 
I think we should create a patch where the events are suppressed and do some heavy testing to find the scope of changes needed by features that may be relying on the initialization events to assess the risk.
Comment 3 Karen Butzke CLA 2008-09-08 15:56:05 EDT 
Created attachment 112010 [details]
proposed patch - HEAD stream

This patch removes the notification when loading profiles.
Comment 4 Larry Dunnell CLA 2008-09-10 14:53:20 EDT 
We had two teams test the patch and found no regressions.  We would like to see the patch applied.
Comment 5 Brian Fitzpatrick CLA 2008-09-10 15:01:50 EDT 
So long as this fixes the original issue found with the JPA stuff causing a thread deadlock, I'm good with delivering this fix.

Karen, is that good with you?
Comment 6 Karen Butzke CLA 2008-09-10 15:05:58 EDT 
I created the patch, so yes, I'm good with it ;)
Comment 7 Brian Fitzpatrick CLA 2008-09-10 16:13:10 EDT 
Very true! 

Let me query the PMC for approval, but we should be able to get this in.
Comment 8 Brian Fitzpatrick CLA 2008-09-10 16:43:20 EDT 
Patch delivered to o.e.d.connectivity as tag v200809110442
Comment 9 Brian Fitzpatrick CLA 2008-09-10 16:43:57 EDT 
Should appear in the 9/11 or 9/12 build