GP-690: Protecting model/manager threads from user callbacks.

This commit is contained in:
Dan
2021-02-12 14:59:44 -05:00
parent 2ed29f5693
commit 56f3f5c656
7 changed files with 87 additions and 24 deletions
@@ -195,8 +195,7 @@ public class DelegateGadpClientTargetObject implements GadpClientTargetObject {
private final List<Class<? extends TargetObject>> ifaces;
private final GadpEventHandlerMap eventHandlers;
private final GadpAttributeChangeCallbackMap attributeChangeCallbacks;
protected final ListenerSet<TargetObjectListener> listeners =
new ListenerSet<>(TargetObjectListener.class);
protected final ListenerSet<TargetObjectListener> listeners;
// TODO: Use path element comparators?
protected final Map<String, TargetObjectRef> elements = new TreeMap<>();
@@ -210,6 +209,7 @@ public class DelegateGadpClientTargetObject implements GadpClientTargetObject {
public DelegateGadpClientTargetObject(GadpClient client, List<String> path, String typeHint,
List<String> ifaceNames, List<Class<? extends TargetObject>> ifaces,
List<Class<? extends TargetObject>> mixins) {
this.listeners = new ListenerSet<>(TargetObjectListener.class, client.getClientExecutor());
this.state = new ProxyState(client, path);
this.hash = computeHashCode();
this.cleanable = GadpClient.CLEANER.register(this, state);
@@ -20,8 +20,7 @@ import java.io.IOException;
import java.lang.ref.Cleaner;
import java.nio.channels.*;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -273,8 +272,9 @@ public class GadpClient implements DebuggerObjectModel {
protected XmlSchemaContext schemaContext;
protected TargetObjectSchema rootSchema;
protected final Executor clientExecutor = Executors.newSingleThreadExecutor();
protected final ListenerSet<DebuggerModelListener> listenersClient =
new ListenerSet<>(DebuggerModelListener.class);
new ListenerSet<>(DebuggerModelListener.class, clientExecutor);
protected final TriConsumer<ChannelState, ChannelState, DebuggerModelClosedReason> listenerForChannelState =
this::channelStateChanged;
protected final MessagePairingCache messageMatcher = new MessagePairingCache();
@@ -836,4 +836,9 @@ public class GadpClient implements DebuggerObjectModel {
proxy.getDelegate().doClearCaches();
}
}
@Override
public Executor getClientExecutor() {
return clientExecutor;
}
}
@@ -18,6 +18,7 @@ package ghidra.dbg;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import ghidra.async.AsyncUtils;
import ghidra.async.TypeSpec;
@@ -501,4 +502,11 @@ public interface DebuggerObjectModel {
Msg.error(origin, message, ex);
}
}
/**
* Get the executor used to invoke client callback routines
*
* @return the executor
*/
Executor getClientExecutor();
}
@@ -15,12 +15,16 @@
*/
package ghidra.dbg.agent;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import ghidra.dbg.DebuggerModelListener;
import ghidra.util.datastruct.ListenerSet;
public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectModel {
protected final Executor clientExecutor = Executors.newSingleThreadExecutor();
protected final ListenerSet<DebuggerModelListener> listeners =
new ListenerSet<>(DebuggerModelListener.class);
new ListenerSet<>(DebuggerModelListener.class, clientExecutor);
@Override
public void addModelListener(DebuggerModelListener listener) {
@@ -31,4 +35,9 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
public void removeModelListener(DebuggerModelListener listener) {
listeners.remove(listener);
}
@Override
public Executor getClientExecutor() {
return clientExecutor;
}
}
@@ -55,11 +55,11 @@ public abstract class AbstractTargetObject<P extends TargetObject>
protected boolean valid = true;
protected final ListenerSet<TargetObjectListener> listeners =
new ListenerSet<>(TargetObjectListener.class);
protected final ListenerSet<TargetObjectListener> listeners;
public AbstractTargetObject(DebuggerObjectModel model, P parent, String key, String typeHint,
TargetObjectSchema schema) {
this.listeners = new ListenerSet<>(TargetObjectListener.class, model.getClientExecutor());
this.model = model;
this.parent = parent;
this.completedParent = CompletableFuture.completedFuture(parent);
@@ -17,6 +17,7 @@ package ghidra.util.datastruct;
import java.lang.reflect.*;
import java.util.*;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicReference;
import com.google.common.cache.*;
@@ -49,6 +50,13 @@ import ghidra.util.Msg;
* @param <V> the type of listeners
*/
public class ListenerMap<K, P, V extends P> {
public static final Executor CALLING_THREAD = new Executor() {
@Override
public void execute(Runnable command) {
command.run();
}
};
protected static final AtomicReference<Throwable> firstExc = new AtomicReference<>();
protected static void reportError(Object listener, Throwable e) {
@@ -107,27 +115,25 @@ public class ListenerMap<K, P, V extends P> {
if (!ext.isAssignableFrom(l.getClass())) {
continue;
}
try {
method.invoke(l, args);
}
catch (InvocationTargetException e) {
Throwable cause = e.getCause();
for (Class<?> excCls : method.getExceptionTypes()) {
if (excCls.isInstance(e)) {
throw cause;
}
executor.execute(() -> {
try {
method.invoke(l, args);
}
reportError(l, cause);
}
catch (Throwable e) {
reportError(l, e);
}
catch (InvocationTargetException e) {
Throwable cause = e.getCause();
reportError(l, cause);
}
catch (Throwable e) {
reportError(l, e);
}
});
}
return null; // TODO: Assumes void return type
}
}
private final Class<P> iface;
private final Executor executor;
private Map<K, V> map = createMap();
/**
@@ -140,6 +146,23 @@ public class ListenerMap<K, P, V extends P> {
*/
protected final Map<Class<? extends P>, P> extFires = new HashMap<>();
/**
* Construct a new map whose proxy implements the given interface
*
* <P>
* The values in the map must implement the same interface.
*
* <P>
* Callbacks will be serviced by the invoking thread. This may be risking if the invoking thread
* is "precious" to the invoker. There is no guarantee callbacks into client code will complete
* in a timely fashion.
*
* @param iface the interface to multiplex
*/
public ListenerMap(Class<P> iface) {
this(iface, CALLING_THREAD);
}
/**
* Construct a new map whose proxy implements the given interface
*
@@ -149,8 +172,9 @@ public class ListenerMap<K, P, V extends P> {
* @param iface the interface to multiplex
*/
@SuppressWarnings("unchecked")
public ListenerMap(Class<P> iface) {
public ListenerMap(Class<P> iface, Executor executor) {
this.iface = Objects.requireNonNull(iface);
this.executor = executor;
this.fire = (P) Proxy.newProxyInstance(this.getClass().getClassLoader(),
new Class[] { iface }, new ListenerHandler<>(iface));
}
@@ -16,6 +16,7 @@
package ghidra.util.datastruct;
import java.util.Map;
import java.util.concurrent.Executor;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.RemovalNotification;
@@ -26,6 +27,7 @@ import com.google.common.cache.RemovalNotification;
* @param <E> the type of multiplexed listeners
*/
public class ListenerSet<E> {
public static final Executor CALLING_THREAD = ListenerMap.CALLING_THREAD;
private final ListenerMap<E, E, E> map;
/**
@@ -36,10 +38,25 @@ public class ListenerSet<E> {
/**
* Construct a new set whose elements and proxy implement the given interface
*
* <p>
* Callbacks will be serviced by the invoking thread. This may be risking if the invoking thread
* is "precious" to the invoker. There is no guarantee callbacks into client code will complete
* in a timely fashion.
*
* @param iface the interface to multiplex
*/
public ListenerSet(Class<E> iface) {
map = new ListenerMap<E, E, E>(iface) {
this(iface, CALLING_THREAD);
}
/**
* Construct a new set whose elements and proxy implement the given interface
*
* @param iface the interface to multiplex
* @param executor an executor for servicing callbacks
*/
public ListenerSet(Class<E> iface, Executor executor) {
map = new ListenerMap<E, E, E>(iface, executor) {
@Override
protected Map<E, E> createMap() {
return ListenerSet.this.createMap();