Issue #346: Convert GFE to use identity-based ListenerLists, increase

synchronization in Parm/VCParm to prevent leaking of VCParms through
ListenerLists.

Former-commit-id: 5fc74409511903afa97949bc3cd1b3acf91c35e0
This commit is contained in:
David Gillingham 2012-03-02 13:22:46 -06:00
parent 787aed90f8
commit 0c0f659896
8 changed files with 85 additions and 42 deletions

View file

@ -89,6 +89,7 @@ import com.raytheon.viz.gfe.core.parm.vcparm.VCModule;
* dispose method. * dispose method.
* 02/23/2012 #346 dgilling Ensure all Parms are disposed when calling * 02/23/2012 #346 dgilling Ensure all Parms are disposed when calling
* dispose method. * dispose method.
* 03/01/2012 #346 dgilling Use identity-based ListenerLists.
* *
* </pre> * </pre>
* *
@ -246,12 +247,14 @@ public abstract class AbstractParmManager implements IParmManager {
protected AbstractParmManager(final DataManager dataManager) { protected AbstractParmManager(final DataManager dataManager) {
this.dataManager = dataManager; this.dataManager = dataManager;
this.parms = new RWLArrayList<Parm>(); this.parms = new RWLArrayList<Parm>();
this.displayedParmListListeners = new ListenerList(); this.displayedParmListListeners = new ListenerList(
this.parmListChangedListeners = new ListenerList(); ListenerList.IDENTITY);
this.systemTimeRangeChangedListeners = new ListenerList(); this.parmListChangedListeners = new ListenerList(ListenerList.IDENTITY);
this.availableSourcesListeners = new ListenerList(); this.systemTimeRangeChangedListeners = new ListenerList(
this.newModelListeners = new ListenerList(); ListenerList.IDENTITY);
this.parmIdChangedListeners = new ListenerList(); this.availableSourcesListeners = new ListenerList(ListenerList.IDENTITY);
this.newModelListeners = new ListenerList(ListenerList.IDENTITY);
this.parmIdChangedListeners = new ListenerList(ListenerList.IDENTITY);
// Get virtual parm definitions // Get virtual parm definitions
vcModules = initVirtualCalcParmDefinitions(); vcModules = initVirtualCalcParmDefinitions();

View file

@ -43,10 +43,11 @@ import com.raytheon.viz.gfe.Activator;
* *
* <pre> * <pre>
* SOFTWARE HISTORY * SOFTWARE HISTORY
* Date Ticket# Engineer Description * Date Ticket# Engineer Description
* ------------ ---------- ----------- -------------------------- * ------------ ---------- ----------- --------------------------
* Jun 11, 2008 chammack Initial creation * Jun 11, 2008 chammack Initial creation
* Sep 3, 2008 1448 chammack Implement refactored interface * Sep 03, 2008 1448 chammack Implement refactored interface
* Mar 01, 2012 #346 dgilling Use identity-based ListenerLists.
* </pre> * </pre>
* *
* @author chammack * @author chammack
@ -68,7 +69,7 @@ public class NotificationRouter implements INotificationObserver {
*/ */
public NotificationRouter(String siteID) { public NotificationRouter(String siteID) {
this.siteID = siteID; this.siteID = siteID;
this.observers = new ListenerList(); this.observers = new ListenerList(ListenerList.IDENTITY);
this.observers this.observers
.add(new AbstractGFENotificationObserver<UserMessageNotification>( .add(new AbstractGFENotificationObserver<UserMessageNotification>(
UserMessageNotification.class) { UserMessageNotification.class) {
@ -178,6 +179,7 @@ public class NotificationRouter implements INotificationObserver {
/** /**
* Notification that a message has arrived * Notification that a message has arrived
*/ */
@Override
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public void notificationArrived(NotificationMessage[] messages) { public void notificationArrived(NotificationMessage[] messages) {
// If DataManager is not initialized yet, do not start listening // If DataManager is not initialized yet, do not start listening

View file

@ -71,6 +71,7 @@ import com.raytheon.viz.gfe.core.griddata.IGridData;
* 01/29/08 #1271 njensen Rewrote populateGridFromData() * 01/29/08 #1271 njensen Rewrote populateGridFromData()
* to use IFPClient * to use IFPClient
* 02/23/12 #346 dgilling Implement a dispose method. * 02/23/12 #346 dgilling Implement a dispose method.
* 03/01/12 #346 dgilling Re-order dispose method.
* *
* </pre> * </pre>
* *
@ -122,8 +123,8 @@ public class DbParm extends Parm {
*/ */
@Override @Override
public void dispose() { public void dispose() {
super.dispose();
looseLocks(); looseLocks();
super.dispose();
} }
// -- private // -- private

View file

@ -173,6 +173,9 @@ import com.vividsolutions.jts.geom.Coordinate;
* 06/08/2009 #2159 rjpeter Fixed undo. * 06/08/2009 #2159 rjpeter Fixed undo.
* 02/23/2012 #346 dgilling Implement a dispose method to mimic * 02/23/2012 #346 dgilling Implement a dispose method to mimic
* AWIPS1 use of C++ destructor. * AWIPS1 use of C++ destructor.
* 03/02/2012 #346 dgilling Create a disposed flag to help ensure
* no interaction with Parms after dispose
* is called.
* </pre> * </pre>
* *
* @author chammack * @author chammack
@ -215,6 +218,8 @@ public abstract class Parm implements Comparable<Parm> {
protected String officeType; protected String officeType;
protected boolean disposed;
/** /**
* The create from scratch mode * The create from scratch mode
*/ */
@ -348,9 +353,15 @@ public abstract class Parm implements Comparable<Parm> {
if (this.officeType == null) { if (this.officeType == null) {
this.officeType = dataManager.getOfficeType(); this.officeType = dataManager.getOfficeType();
} }
this.disposed = false;
} }
public void dispose() { public void dispose() {
synchronized (this) {
this.disposed = true;
}
if (isModified()) { if (isModified()) {
statusHandler.warn("Destroying parm " + getParmID().toString() statusHandler.warn("Destroying parm " + getParmID().toString()
+ " with modified data."); + " with modified data.");

View file

@ -50,12 +50,13 @@ import com.raytheon.viz.gfe.core.wxvalue.WxValue;
* *
* <pre> * <pre>
* SOFTWARE HISTORY * SOFTWARE HISTORY
* Date Ticket# Engineer Description * Date Ticket# Engineer Description
* ------------ ---------- ----------- -------------------------- * ------------ ---------- ----------- --------------------------
* Jun 13, 2008 chammack Initial creation * Jun 13, 2008 chammack Initial creation
* Sep 01, 2009 #2788 randerso Changed listener lists to sets to prevent * Sep 01, 2009 #2788 randerso Changed listener lists to sets to prevent
* multiple registration * multiple registration
* Feb 23, 2012 #346 dgilling Implement clearParmListeners. * Feb 23, 2012 #346 dgilling Implement clearParmListeners.
* Mar 01, 2012 #346 dgilling Use identity-based ListenerLists.
* *
* </pre> * </pre>
* *
@ -88,16 +89,23 @@ public class ParmListeners {
private final JobPool notificationPool; private final JobPool notificationPool;
protected ParmListeners(JobPool pool) { protected ParmListeners(JobPool pool) {
this.gridChangedListeners = new ListenerList(); this.gridChangedListeners = new ListenerList(ListenerList.IDENTITY);
this.parmInventoryChangedListeners = new ListenerList(); this.parmInventoryChangedListeners = new ListenerList(
this.parmIDChangedListeners = new ListenerList(); ListenerList.IDENTITY);
this.selectionTimeRangeChangedListeners = new ListenerList(); this.parmIDChangedListeners = new ListenerList(ListenerList.IDENTITY);
this.parameterSelectionChangedListeners = new ListenerList(); this.selectionTimeRangeChangedListeners = new ListenerList(
this.combineModeChangedListeners = new ListenerList(); ListenerList.IDENTITY);
this.vectorModeChangedListeners = new ListenerList(); this.parameterSelectionChangedListeners = new ListenerList(
this.pickupValueChangedListeners = new ListenerList(); ListenerList.IDENTITY);
this.colorTableModifiedListeners = new ListenerList(); this.combineModeChangedListeners = new ListenerList(
this.lockTableChangedListeners = new ListenerList(); ListenerList.IDENTITY);
this.vectorModeChangedListeners = new ListenerList(
ListenerList.IDENTITY);
this.pickupValueChangedListeners = new ListenerList(
ListenerList.IDENTITY);
this.colorTableModifiedListeners = new ListenerList(
ListenerList.IDENTITY);
this.lockTableChangedListeners = new ListenerList(ListenerList.IDENTITY);
this.notificationPool = pool; this.notificationPool = pool;
} }

View file

@ -64,6 +64,9 @@ import com.raytheon.viz.gfe.core.parm.vcparm.VCModule.VCInventory;
* data. Also, remove overridden * data. Also, remove overridden
* finalize function. * finalize function.
* Feb 23, 2012 #346 dgilling Implement a dispose method. * Feb 23, 2012 #346 dgilling Implement a dispose method.
* Mar 02, 2012 #346 dgilling Use Parm's new disposed flag to
* prevent leaks through
* ListenerLists.
* *
* </pre> * </pre>
* *
@ -138,6 +141,12 @@ public class VCParm extends VParm implements IParmListChangedListener,
*/ */
@Override @Override
public void gridDataChanged(final ParmID parmId, final TimeRange validTime) { public void gridDataChanged(final ParmID parmId, final TimeRange validTime) {
synchronized (this) {
if (disposed) {
return;
}
}
// statusHandler.handle(Priority.DEBUG, "gridDataChanged for: " + parmId // statusHandler.handle(Priority.DEBUG, "gridDataChanged for: " + parmId
// + " " + validTime); // + " " + validTime);
@ -167,7 +176,6 @@ public class VCParm extends VParm implements IParmListChangedListener,
} }
} }
} }
} }
/* /*
@ -182,13 +190,22 @@ public class VCParm extends VParm implements IParmListChangedListener,
@Override @Override
public void parmListChanged(final Parm[] parms, Parm[] deletions, public void parmListChanged(final Parm[] parms, Parm[] deletions,
Parm[] additions) { Parm[] additions) {
// statusHandler.handle(Priority.DEBUG,
// "ParmListChangedMsg received: ");
System.out.println("ParmListChangedMsg received: "
+ getParmID().toString());
registerParmClients(parms, true); // forcing access to the disposed variable and subsequent
// registration/unregistation of listeners through this synchronized
// block seems to prevent VCParm objects being leaked through outdated
// listener list copies
synchronized (this) {
if (disposed) {
return;
}
// statusHandler.handle(Priority.DEBUG,
// "ParmListChangedMsg received: ");
// System.out.println("ParmListChangedMsg received: "
// + getParmID().toString());
registerParmClients(parms, true);
}
} }
/* /*
@ -208,8 +225,13 @@ public class VCParm extends VParm implements IParmListChangedListener,
// approach. // approach.
// statusHandler.debug("ParmInventoryChanged notification for: " // statusHandler.debug("ParmInventoryChanged notification for: "
// + getParmID().toString()); // + getParmID().toString());
System.out.println("ParmInventoryChanged notification for: " // System.out.println("ParmInventoryChanged notification for: "
+ getParmID().toString()); // + getParmID().toString());
synchronized (this) {
if (disposed) {
return;
}
}
recalcInventory(true); recalcInventory(true);
} }
@ -467,9 +489,6 @@ public class VCParm extends VParm implements IParmListChangedListener,
} }
private void registerPC(Parm parm) { private void registerPC(Parm parm) {
System.out.println(getParmID().toString() + " is registering "
+ parm.toString());
parm.parmListeners.addGridChangedListener(this); parm.parmListeners.addGridChangedListener(this);
parm.parmListeners.addParmInventoryChangedListener(this); parm.parmListeners.addParmInventoryChangedListener(this);
@ -482,9 +501,6 @@ public class VCParm extends VParm implements IParmListChangedListener,
} }
private void unregisterPC(Parm parm) { private void unregisterPC(Parm parm) {
System.out.println(getParmID().toString() + " is unregistering "
+ parm.toString());
parm.parmListeners.removeGridChangedListener(this); parm.parmListeners.removeGridChangedListener(this);
parm.parmListeners.removeParmInventoryChangedListener(this); parm.parmListeners.removeParmInventoryChangedListener(this);

View file

@ -42,6 +42,7 @@ import com.raytheon.viz.gfe.core.msgs.ISmartToolInventoryChanged;
* Date Ticket# Engineer Description * Date Ticket# Engineer Description
* ------------ ---------- ----------- -------------------------- * ------------ ---------- ----------- --------------------------
* Jan 19, 2010 njensen Initial creation * Jan 19, 2010 njensen Initial creation
* Mar 01, 2012 #346 dgilling Use identity-based ListenerLists.
* *
* </pre> * </pre>
* *
@ -58,7 +59,7 @@ public class SmartToolUIController extends SmartToolController {
throws JepException { throws JepException {
super(filePath, anIncludePath, classLoader, dataManager); super(filePath, anIncludePath, classLoader, dataManager);
invChangedListeners = new ListenerList(); invChangedListeners = new ListenerList(ListenerList.IDENTITY);
} }
/** /**

View file

@ -46,7 +46,8 @@ import com.raytheon.viz.gfe.tasks.AbstractGfeTask.TaskStatus;
* *
* Date Ticket# Engineer Description * Date Ticket# Engineer Description
* ------------ ---------- ----------- -------------------------- * ------------ ---------- ----------- --------------------------
* Apr 7, 2011 randerso Initial creation * Apr 07, 2011 randerso Initial creation
* Mar 03, 2012 #346 dgilling Use identity-based ListenerLists.
* *
* </pre> * </pre>
* *
@ -70,7 +71,7 @@ public class TaskScheduler extends Job {
protected TaskScheduler() { protected TaskScheduler() {
super("Task Manager Job"); super("Task Manager Job");
listeners = new ListenerList(); listeners = new ListenerList(ListenerList.IDENTITY);
PythonPreferenceStore prefs = Activator.getDefault() PythonPreferenceStore prefs = Activator.getDefault()
.getPreferenceStore(); .getPreferenceStore();