Skip to content

Commit

Permalink
Fixes a potential deadlock; it's not safe to message likely super nod… (
Browse files Browse the repository at this point in the history
#56)

* Fixes a potential deadlock; it's not safe to message likely super nodes while holding the lock.

* I think this is what @nguyenhuy is looking for :)

* Fix up CHANGELOG.md
  • Loading branch information
garrettmoon committed Apr 24, 2017
1 parent e45c686 commit 2d6ccdb
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## master

* Add your own contributions to the next release on the line below this with your name.
Philipp Smorygo <[email protected]> Fix `__has_include` check in ASLog.h
Fix `__has_include` check in ASLog.h [Philipp Smorygo]([email protected])
Fix potential deadlock in ASControlNode [Garrett Moon](https://github.com/garrettmoon)
34 changes: 22 additions & 12 deletions Source/ASControlNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -420,34 +420,44 @@ - (void)removeTarget:(id)target action:(SEL)action forControlEvents:(ASControlNo

- (void)sendActionsForControlEvents:(ASControlNodeEvent)controlEvents withEvent:(UIEvent *)event
{
ASDisplayNodeAssertMainThread(); //We access self.view below, it's not safe to call this off of main.
NSParameterAssert(controlEvents != 0);

ASDN::MutexLocker l(_controlLock);
NSMutableArray *resolvedEventTargetActionArray = [[NSMutableArray<ASControlTargetAction *> alloc] init];

_controlLock.lock();

// Enumerate the events in the mask, invoking the target-action pairs for each.
_ASEnumerateControlEventsIncludedInMaskWithBlock(controlEvents, ^
(ASControlNodeEvent controlEvent)
{
// Use a copy to itereate, the action perform could call remove causing a mutation crash.
NSArray *eventTargetActionArray = [_controlEventDispatchTable[_ASControlNodeEventKeyForControlEvent(controlEvent)] copy];

// Iterate on each target action pair
for (ASControlTargetAction *targetAction in eventTargetActionArray) {
SEL action = targetAction.action;
id responder = targetAction.target;
for (ASControlTargetAction *targetAction in _controlEventDispatchTable[_ASControlNodeEventKeyForControlEvent(controlEvent)]) {
ASControlTargetAction *resolvedTargetAction = [[ASControlTargetAction alloc] init];
resolvedTargetAction.action = targetAction.action;
resolvedTargetAction.target = targetAction.target;

// NSNull means that a nil target was set, so start at self and travel the responder chain
if (!responder && targetAction.createdWithNoTarget) {
if (!resolvedTargetAction.target && targetAction.createdWithNoTarget) {
// if the target cannot perform the action, travel the responder chain to try to find something that does
responder = [self.view targetForAction:action withSender:self];
resolvedTargetAction.target = [self.view targetForAction:resolvedTargetAction.action withSender:self];
}

if (resolvedTargetAction.target) {
[resolvedEventTargetActionArray addObject:resolvedTargetAction];
}
}
});

_controlLock.unlock();

//We don't want to hold the lock while calling out, we could potentially walk up the ownership tree causing a deadlock.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
[responder performSelector:action withObject:self withObject:event];
for (ASControlTargetAction *targetAction in resolvedEventTargetActionArray) {
[targetAction.target performSelector:targetAction.action withObject:self withObject:event];
}
#pragma clang diagnostic pop
}
});
}

#pragma mark - Convenience
Expand Down

0 comments on commit 2d6ccdb

Please sign in to comment.