Crash on ln 363 of DCRoundSwitch.m, due to deallocated target

[jaredegan]

First of, thanks for such a great class!

There's a possibility of a crash when DCRoundSwitch calls sendActionsForControlEvents, if one of the targets of the switch as been deallocated. It's caused by delaying the sendActions... call to after the animations have completed. I assume that this is matching UISwitches behavior, but I don't think UISwitch has this crash. One way to reproduce it is to have a view controller be a DCRoundSwitch target, then flip the switch and hit back as soon as you can afterward, causing the view controller to be deallocated.

Exception:

*** -[SettingsController performSelector:withObject:withObject:]: message sent to deallocated instance 0x7a84020

Code:

[CATransaction setCompletionBlock:^{
    [self removeLayerMask];
    ignoreTap = NO;

    // send the action here so it get's sent at the end of the animations
    if (previousOn != on && !ignoreControlEvents)
        [self sendActionsForControlEvents:UIControlEventValueChanged]; // (Crashes here)
}];

This can be fixed in the dealloc of the targets, by having the target remove itself from the switch, like so:

- (void)dealloc {
    [self.mySwitch removeTarget:self action:@selector(switchChanged:) forControlEvents:UIControlEventAllEvents];
}

I'm not sure what the fix should be size I do believe the value changed event for UISwitches gets fired after the animation completes. At the least instructions to prevent this crash should be added to the docs.

[jaredegan]

Actually, a better fix may be this line, which removes all targets without needing to specify the selectors:

[self.mySwitch removeTarget:nil action:NULL forControlEvents:UIControlEventAllEvents];

[domesticcatsoftware]

Yep I did confirm this. I ended up fixing by retaining all the targets then releasing them after sending the message.

If you look at how UISwitch operates that’s how it seems to work also: objects that are added as targets don’t get released until the switch finishes it’s animation and fires the action.

By the way, easy way to test this: use the alt key in the simulator to bring up the second “finger”, then arrange it so you tap the back button to pop your view controller as well as fire a switch at the same time.

[triberger]

Hello,

In (void)setOn:(BOOL)newOn animated:(BOOL)animated ignoreControlEvents:(BOOL)ignoreControlEvents instead of using [self allTargets] you should better do a copy of the allTargets NSSet and use it instead.

Example:

DCRoundSwitch *pSubscribeSwitch=[[DCRoundSwitch alloc] initWithFrame:CGRectMake(0.0f,0.0f,80.0f,17.0f)]; pSubscribeSwitch.on=_bSubscribeComment; [pSubscribeSwitch addTarget:self action:@selector(subscribe:) forControlEvents:UIControlEventValueChanged];

In this case the allTargets NSSet is modified between the call to :

[[self allTargets] makeObjectsPerformSelector:@selector(retain)];

and

[[self allTargets] makeObjectsPerformSelector:@selector(release)];

leading to a crash because the target was released but not retained.

Making a copy of the allTargets NSSet before calling [[self allTargets] makeObjectsPerformSelector:@selector(retain)]; and using this copy afterwards corrects the bug.

(and sorry for my bad English).

[niklassaers]

I just got the same crash from the version available through CocoaPods.

[xingzhisg]

@sfeldis was correct. To avoid the crash I replaced the followings:

//[[self allTargets] makeObjectsPerformSelector:@selector(retain)];
__block NSSet * targets = [[self allTargets] copy];

and

   //[[self allTargets] makeObjectsPerformSelector:@selector(release)];
 [targets release];