eirikbakke commented on a change in pull request #652:
URL: https://github.com/apache/netbeans/pull/652#discussion_r479722069
##########
File path: openide.awt/src/org/openide/awt/ContextAction.java
##########
@@ -181,36 +317,90 @@ public Performer(
ContextActionEnabler<Data> e
) {
Map<Object, Object> map = new HashMap<Object, Object>();
- map.put("delegate", p);
- map.put("enabler", e);
+ map.put("delegate", p); // NOI18N
+ map.put("enabler", e); // NOI18N
this.delegate = map;
}
-
- @SuppressWarnings("unchecked")
- public void actionPerformed(
- ActionEvent ev, List<? extends Data> data, Lookup.Provider
everything
- ) {
- Object obj = delegate.get("delegate"); // NOI18N
- if (obj instanceof ContextActionPerformer) {
- ContextActionPerformer<Data> perf =
(ContextActionPerformer<Data>)obj;
- perf.actionPerformed(ev, data);
- return;
+
+ void clear() {
+ stopListeners();
+ Reference r = instDelegate;
+ instDelegate = null;
+ if (r != null) {
+ Object o = r.get();
+ if (o instanceof Performer) {
+ ((Performer)o).clear();
+ }
}
- if (obj instanceof Performer) {
- Performer<Data> perf = (Performer<Data>)obj;
- perf.actionPerformed(ev, data, everything);
- return;
+ }
+
+ void attach(ContextAction a) {
+ this.owner = new WeakReference<>(a);
+ }
+
+ /**
+ * Creates a delegate.
+ * @param everything
+ * @param data
+ * @return
+ */
+ Object delegate(Lookup.Provider everything, List<?> data) {
+ return delegate0(everything, data, true);
+ }
+
+ private Object delegate0(Lookup.Provider everything, List<?> data,
boolean getAction) {
+ Object d = instDelegate != null ? instDelegate.get() : null;
Review comment:
There seems to be a bug here, which I bumped into this month in my
NetBeans Platform application. It seems that delegate0 will use a cached copy
of the instDelegate--but what if the contents of the lookup (the data
parameter) has changed?
I see there's a clear() method that's supposed to reset instDelegate in
certain cases, but I'm not sure how it works--and it does not seem to be called
in all cases where the lookup has changed.
The bug is insidious because instDelegate is a weak reference, so the buggy
state disappears on garbage collection.
In my platform application, the action in question is declared like this:
```
@ActionID(category = "Ultorg", id = "com.ultorg.actionids.FieldsAction")
@ActionRegistration(displayName = "#CTL_FieldsAction")
@Messages("CTL_FieldsAction=Fields & Joins...")
public class FieldsAction implements ActionListener {
private final ToolboxInvoker invoker;
public FieldsAction(ToolboxInvoker invoker) {
Preconditions.checkNotNull(invoker);
this.invoker = invoker;
}
@Override
public void actionPerformed(ActionEvent e) {
invoker.invoke();
}
}
```
The ToolboxInvoker is exposed in the lookup of a TopComponent subclass.
Invoking the action from the keyboard will occasionally trigger the wrong
FieldsAction instance. Once this happens, it will usually happen again and
again if the keyboard shortcut is pressed again, until a garbage collection
happens (tested by adding a keyboard shortcut to the Garbage Collect action).
The cause of the bug seems to be incorrect state kept around in the
instDelegate variable (confirmed in the debugger).
Is this the same as the bug described in
https://issues.apache.org/jira/browse/NETBEANS-1985 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
