Commit b29e8bbbc8d337bb12c14e81aa18ee2565b07830
1 parent
bbee82bb
adding injection order checks to EventInjectionTransformer to prevent listeners …
…being appended after event injection
Showing
4 changed files
with
155 additions
and
51 deletions
java/common/com/mumfrey/liteloader/transformers/event/Event.java
@@ -93,6 +93,8 @@ public class Event implements Comparable<Event> | @@ -93,6 +93,8 @@ public class Event implements Comparable<Event> | ||
93 | 93 | ||
94 | protected String eventInfoClass; | 94 | protected String eventInfoClass; |
95 | 95 | ||
96 | + protected Set<MethodInfo> pendingInjections; | ||
97 | + | ||
96 | Event(String name, boolean cancellable, int priority) | 98 | Event(String name, boolean cancellable, int priority) |
97 | { | 99 | { |
98 | this.name = name.toLowerCase(); | 100 | this.name = name.toLowerCase(); |
@@ -226,6 +228,38 @@ public class Event implements Comparable<Event> | @@ -226,6 +228,38 @@ public class Event implements Comparable<Event> | ||
226 | this.method = null; | 228 | this.method = null; |
227 | } | 229 | } |
228 | 230 | ||
231 | + void addPendingInjection(MethodInfo targetMethod) | ||
232 | + { | ||
233 | + if (this.pendingInjections == null) | ||
234 | + { | ||
235 | + this.pendingInjections = new HashSet<MethodInfo>(); | ||
236 | + } | ||
237 | + | ||
238 | + this.pendingInjections.add(targetMethod); | ||
239 | + } | ||
240 | + | ||
241 | + void notifyInjected(String method, String desc, String className) | ||
242 | + { | ||
243 | + MethodInfo thisInjection = null; | ||
244 | + | ||
245 | + if (this.pendingInjections != null) | ||
246 | + { | ||
247 | + for (MethodInfo pendingInjection : this.pendingInjections) | ||
248 | + { | ||
249 | + if (pendingInjection.matches(method, desc, className)) | ||
250 | + { | ||
251 | + thisInjection = pendingInjection; | ||
252 | + break; | ||
253 | + } | ||
254 | + } | ||
255 | + } | ||
256 | + | ||
257 | + if (thisInjection != null) | ||
258 | + { | ||
259 | + this.pendingInjections.remove(thisInjection); | ||
260 | + } | ||
261 | + } | ||
262 | + | ||
229 | /** | 263 | /** |
230 | * Pre-flight check | 264 | * Pre-flight check |
231 | * | 265 | * |
@@ -371,6 +405,11 @@ public class Event implements Comparable<Event> | @@ -371,6 +405,11 @@ public class Event implements Comparable<Event> | ||
371 | throw new IllegalArgumentException("Descriptor is not allowed for listener methods"); | 405 | throw new IllegalArgumentException("Descriptor is not allowed for listener methods"); |
372 | } | 406 | } |
373 | 407 | ||
408 | + if (this.pendingInjections != null && this.pendingInjections.size() == 0) | ||
409 | + { | ||
410 | + throw new EventAlreadyInjectedException("The event " + this.name + " was already injected and has 0 pending injections, addListener() is not allowed at this point"); | ||
411 | + } | ||
412 | + | ||
374 | this.listeners.add(listener); | 413 | this.listeners.add(listener); |
375 | 414 | ||
376 | return this; | 415 | return this; |
java/common/com/mumfrey/liteloader/transformers/event/EventAlreadyInjectedException.java
0 → 100644
1 | +package com.mumfrey.liteloader.transformers.event; | ||
2 | + | ||
3 | +public class EventAlreadyInjectedException extends RuntimeException | ||
4 | +{ | ||
5 | + private static final long serialVersionUID = 1L; | ||
6 | + | ||
7 | + public EventAlreadyInjectedException(String message) | ||
8 | + { | ||
9 | + super(message); | ||
10 | + } | ||
11 | + | ||
12 | + public EventAlreadyInjectedException(Throwable cause) | ||
13 | + { | ||
14 | + super(cause); | ||
15 | + } | ||
16 | + | ||
17 | + public EventAlreadyInjectedException(String message, Throwable cause) | ||
18 | + { | ||
19 | + super(message, cause); | ||
20 | + } | ||
21 | +} |
java/common/com/mumfrey/liteloader/transformers/event/EventInjectionTransformer.java
@@ -77,7 +77,14 @@ public abstract class EventInjectionTransformer extends ClassTransformer | @@ -77,7 +77,14 @@ public abstract class EventInjectionTransformer extends ClassTransformer | ||
77 | EventInjectionTransformer.master = this; | 77 | EventInjectionTransformer.master = this; |
78 | } | 78 | } |
79 | 79 | ||
80 | - this.addEvents(); | 80 | + try |
81 | + { | ||
82 | + this.addEvents(); | ||
83 | + } | ||
84 | + catch (Exception ex) | ||
85 | + { | ||
86 | + ex.printStackTrace(); | ||
87 | + } | ||
81 | } | 88 | } |
82 | 89 | ||
83 | /** | 90 | /** |
@@ -120,6 +127,8 @@ public abstract class EventInjectionTransformer extends ClassTransformer | @@ -120,6 +127,8 @@ public abstract class EventInjectionTransformer extends ClassTransformer | ||
120 | this.addEvent(event, targetMethod.owner, targetMethod.sigSrg, injectionPoint); | 127 | this.addEvent(event, targetMethod.owner, targetMethod.sigSrg, injectionPoint); |
121 | this.addEvent(event, targetMethod.ownerObf, targetMethod.sigObf, injectionPoint); | 128 | this.addEvent(event, targetMethod.ownerObf, targetMethod.sigObf, injectionPoint); |
122 | 129 | ||
130 | + event.addPendingInjection(targetMethod); | ||
131 | + | ||
123 | return event; | 132 | return event; |
124 | } | 133 | } |
125 | 134 | ||
@@ -215,65 +224,100 @@ public abstract class EventInjectionTransformer extends ClassTransformer | @@ -215,65 +224,100 @@ public abstract class EventInjectionTransformer extends ClassTransformer | ||
215 | Map<Event, InjectionPoint> methodInjections = mappings.get(signature); | 224 | Map<Event, InjectionPoint> methodInjections = mappings.get(signature); |
216 | if (methodInjections != null) | 225 | if (methodInjections != null) |
217 | { | 226 | { |
218 | - ReadOnlyInsnList insns = new ReadOnlyInsnList(method.instructions); | ||
219 | - Map<AbstractInsnNode, Set<Event>> injectionPoints = new LinkedHashMap<AbstractInsnNode, Set<Event>>(); | ||
220 | - Collection<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32); | ||
221 | - for (Entry<Event, InjectionPoint> eventEntry : methodInjections.entrySet()) | ||
222 | - { | ||
223 | - Event event = eventEntry.getKey(); | ||
224 | - event.attach(method); | ||
225 | - InjectionPoint injectionPoint = eventEntry.getValue(); | ||
226 | - nodes.clear(); | ||
227 | - if (injectionPoint.find(method.desc, insns, nodes, event)) | ||
228 | - { | ||
229 | - for (AbstractInsnNode node : nodes) | ||
230 | - { | ||
231 | - Set<Event> nodeEvents = injectionPoints.get(node); | ||
232 | - if (nodeEvents == null) | ||
233 | - { | ||
234 | - nodeEvents = new TreeSet<Event>(); | ||
235 | - injectionPoints.put(node, nodeEvents); | ||
236 | - } | ||
237 | - | ||
238 | - nodeEvents.add(event); | ||
239 | - } | ||
240 | - } | ||
241 | - } | ||
242 | - | ||
243 | - for (Entry<AbstractInsnNode, Set<Event>> injectionPoint : injectionPoints.entrySet()) | ||
244 | - { | ||
245 | - AbstractInsnNode insn = injectionPoint.getKey(); | ||
246 | - Set<Event> events = injectionPoint.getValue(); | 227 | + this.injectIntoMethod(classNode, signature, method, methodInjections); |
228 | + } | ||
229 | + } | ||
230 | + | ||
231 | + if (true || this.runValidator) | ||
232 | + { | ||
233 | + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); | ||
234 | + classNode.accept(new CheckClassAdapter(writer)); | ||
235 | + } | ||
236 | + | ||
237 | + return this.writeClass(classNode); | ||
238 | + } | ||
247 | 239 | ||
248 | - // Injection is cancellable if ANY of the events on this insn are cancellable | ||
249 | - boolean cancellable = false; | ||
250 | - for (Event event : events) | ||
251 | - cancellable |= event.isCancellable(); | ||
252 | - | ||
253 | - Event head = events.iterator().next(); | ||
254 | - MethodNode handler = head.inject(insn, cancellable, this.globalEventID); | 240 | + /** |
241 | + * @param classNode | ||
242 | + * @param signature | ||
243 | + * @param method | ||
244 | + * @param methodInjections | ||
245 | + */ | ||
246 | + void injectIntoMethod(ClassNode classNode, String signature, MethodNode method, Map<Event, InjectionPoint> methodInjections) | ||
247 | + { | ||
248 | + Map<AbstractInsnNode, Set<Event>> injectionPoints = this.findInjectionPoints(classNode, method, methodInjections); | ||
249 | + | ||
250 | + for (Entry<AbstractInsnNode, Set<Event>> injectionPoint : injectionPoints.entrySet()) | ||
251 | + { | ||
252 | + this.injectEventsAt(classNode, method, injectionPoint.getKey(), injectionPoint.getValue()); | ||
253 | + } | ||
255 | 254 | ||
256 | - LiteLoaderLogger.info("Injecting event %s with %d handlers in method %s in class %s", head.getName(), events.size(), method.name, classNode.name.replace('/', '.')); | ||
257 | - | ||
258 | - for (Event event : events) | ||
259 | - event.addToHandler(handler); | ||
260 | - | ||
261 | - this.globalEventID++; | ||
262 | - } | 255 | + for (Event event : methodInjections.keySet()) |
256 | + { | ||
257 | + event.notifyInjected(method.name, method.desc, classNode.name); | ||
258 | + event.detach(); | ||
259 | + } | ||
260 | + } | ||
263 | 261 | ||
264 | - for (Event event : methodInjections.keySet()) | 262 | + /** |
263 | + * @param classNode | ||
264 | + * @param method | ||
265 | + * @param methodInjections | ||
266 | + * @return | ||
267 | + */ | ||
268 | + private Map<AbstractInsnNode, Set<Event>> findInjectionPoints(ClassNode classNode, MethodNode method, Map<Event, InjectionPoint> methodInjections) | ||
269 | + { | ||
270 | + ReadOnlyInsnList insns = new ReadOnlyInsnList(method.instructions); | ||
271 | + Collection<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32); | ||
272 | + Map<AbstractInsnNode, Set<Event>> injectionPoints = new LinkedHashMap<AbstractInsnNode, Set<Event>>(); | ||
273 | + for (Entry<Event, InjectionPoint> eventEntry : methodInjections.entrySet()) | ||
274 | + { | ||
275 | + Event event = eventEntry.getKey(); | ||
276 | + event.attach(method); | ||
277 | + InjectionPoint injectionPoint = eventEntry.getValue(); | ||
278 | + nodes.clear(); | ||
279 | + if (injectionPoint.find(method.desc, insns, nodes, event)) | ||
280 | + { | ||
281 | + for (AbstractInsnNode node : nodes) | ||
265 | { | 282 | { |
266 | - event.detach(); | 283 | + Set<Event> nodeEvents = injectionPoints.get(node); |
284 | + if (nodeEvents == null) | ||
285 | + { | ||
286 | + nodeEvents = new TreeSet<Event>(); | ||
287 | + injectionPoints.put(node, nodeEvents); | ||
288 | + } | ||
289 | + | ||
290 | + nodeEvents.add(event); | ||
267 | } | 291 | } |
268 | } | 292 | } |
269 | } | 293 | } |
270 | 294 | ||
271 | - if (true || this.runValidator) | 295 | + return injectionPoints; |
296 | + } | ||
297 | + | ||
298 | + /** | ||
299 | + * @param classNode | ||
300 | + * @param method | ||
301 | + * @param injectionPoint | ||
302 | + * @param events | ||
303 | + */ | ||
304 | + private void injectEventsAt(ClassNode classNode, MethodNode method, AbstractInsnNode injectionPoint, Set<Event> events) | ||
305 | + { | ||
306 | + // Injection is cancellable if ANY of the events on this insn are cancellable | ||
307 | + boolean cancellable = false; | ||
308 | + for (Event event : events) | ||
309 | + cancellable |= event.isCancellable(); | ||
310 | + | ||
311 | + Event head = events.iterator().next(); | ||
312 | + MethodNode handler = head.inject(injectionPoint, cancellable, this.globalEventID); | ||
313 | + | ||
314 | + LiteLoaderLogger.info("Injecting event %s with %d handlers in method %s in class %s", head.getName(), events.size(), method.name, classNode.name.replace('/', '.')); | ||
315 | + | ||
316 | + for (Event event : events) | ||
272 | { | 317 | { |
273 | - ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); | ||
274 | - classNode.accept(new CheckClassAdapter(writer)); | 318 | + event.addToHandler(handler); |
275 | } | 319 | } |
276 | 320 | ||
277 | - return this.writeClass(classNode); | 321 | + this.globalEventID++; |
278 | } | 322 | } |
279 | } | 323 | } |
280 | \ No newline at end of file | 324 | \ No newline at end of file |
java/common/com/mumfrey/liteloader/transformers/event/MethodInfo.java
@@ -290,7 +290,7 @@ public class MethodInfo | @@ -290,7 +290,7 @@ public class MethodInfo | ||
290 | 290 | ||
291 | public boolean matches(String method, String desc, String className) | 291 | public boolean matches(String method, String desc, String className) |
292 | { | 292 | { |
293 | - if ((className == null || this.owner.equals(className)) && (this.name.equals(method) || this.nameSrg.equals(method))) | 293 | + if ((className == null || this.ownerRef.equals(className)) && (this.name.equals(method) || this.nameSrg.equals(method))) |
294 | { | 294 | { |
295 | return this.desc == null || this.desc.equals(desc); | 295 | return this.desc == null || this.desc.equals(desc); |
296 | } | 296 | } |