race condition when displaying multiple AlertViews

[sberrevoets]

race condition when displaying multiple AlertViews

[sberrevoets]

That would certainly be an issue. If you could give me an exact code example, I will investigate it. Thanks for reporting!

[ubragg]

Sure, I will try to whip up a simple repro case for you if I can find some time in the next few days.

[ubragg]

OK, here is one simple case that shows absolutely nothing:

static SDCAlertView* alertView1;
static SDCAlertView* alertView2;

- (void) viewDidAppear:(BOOL) animated {
  [super viewDidAppear:animated];
  alertView1 = [[SDCAlertView alloc] initWithTitle:@"Title 1"
                                           message:@"Message 1"
                                          delegate:nil
                                 cancelButtonTitle:@"Cancel"
                                 otherButtonTitles:nil];
  alertView2 = [[SDCAlertView alloc] initWithTitle:@"Title 2"
                                           message:@"Message 2"
                                          delegate:nil
                                 cancelButtonTitle:@"Cancel"
                                 otherButtonTitles:nil];
  [alertView1 show];
  [alertView1 dismissWithClickedButtonIndex:0 animated:NO];
  [alertView2 show];
}

A more subtle case is this one:

static SDCAlertView* alertView1;
static SDCAlertView* alertView2;

- (void) viewDidAppear:(BOOL) animated {
  [super viewDidAppear:animated];
  alertView1 = [[SDCAlertView alloc] initWithTitle:@"Title 1"
                                           message:@"Message 1"
                                          delegate:nil
                                 cancelButtonTitle:@"Cancel"
                                 otherButtonTitles:nil];
  alertView2 = [[SDCAlertView alloc] initWithTitle:@"Title 2"
                                           message:@"Message 2"
                                          delegate:nil
                                 cancelButtonTitle:@"Cancel"
                                 otherButtonTitles:nil];
  [alertView1 show];
}

- (IBAction) pressedSignIn:(id) sender {
  [alertView1 dismissWithClickedButtonIndex:0 animated:NO];
  [alertView2 show];
}

In this case the first one shows up when the page loads, but then when I hit the button which is wired up to "pressedSignin", nothing happens. It is easy to come up with more complicated scenarios, but these seem to illustrate the issue pretty well I think.

[sberrevoets]

I see what the problem is, the fix shouldn't be too terribly hard. I'll take a look at it tomorrow.

What's funny, is that even UIAlertView has a bug:

[alertView1 show];
[alertView1 dismissWithClickedButtonIndex:0 animated:NO];

This will cause the tintAdjustmentMode to not change from dimmed to normal.

[ubragg]

Cool, thanks for the quick response!

[sberrevoets]

I've taken a look at it. I have some good news and (potentially) bad news.

The good news is that it's all fixed. Alerts are always shown, no matter in what order you make show/dismiss calls. SDCAlertViewCoordinator uses a queue for all presenting/dismissing related method calls during animation, and after each animation, any next animation will occur.

That also means that a call sequence like this:

[alert1 show];
[alert1 dismissWithClickedButtonIndex:0];
[alert2 show];

will cause alert1 to fade away, including the dimming background view and tint adjustment mode. After that's done, alert2 will be shown, causing the dimming background to fade back in. This may not be the behavior you're looking for, but in my opinion that's the most logical thing to happen.

I know this is not how UIAlertView behaves. I have no idea how Apple handles this--I believe it may be the side-effect of a race condition (as far as I can tell, they happen quite a bit when dealing with this) on their end.

If that is not the behavior you want, I recommend using the alertView:willDismissWithButtonIndex: delegate method. If you show the new alert from within that method, the dimming view will not fade out and the second alert will be presented more naturally.

Before I merge the changes into master, I'd like you to take a look at it. Check out the queuing-issue branch (19436dd), and see if this fixes your problem (keeping in mind what I wrote above). If all's well, I will merge the branch into master.

EDIT: I've also fixed an issue that would not honor the animated argument if you call dismissWithClickedButtonIndex:animated:. Before, it would always dismiss with an animation, but now, if you pass NO, it will just disappear.

[ubragg]

Thanks for the quick turnaround on that! I'll check it out and let you
know, but it might take a couple days before I have time to get to it.

On Sat, Mar 29, 2014 at 2:43 PM, Scott Berrevoets
[email protected]:

I've taken a look at it. I have some good news and (potentially) bad news.

The good news is that it's all fixed. Alerts are always shown, no matter
in what order you make show/dismiss calls. SDCAlertViewCoordinator uses a
queue for all presenting/dismissing related method calls during animation,
and after each animation, any next animation will occur.

That also means that a call sequence like this:

[alert1 show];
[alert1 dismissWithClickedButtonIndex:0];
[alert2 show];

will cause alert1 to fade away, including the dimming background view and
tint adjustment mode. After that's done, alert2 will be shown, causing
the dimming background to fade back in. This may not be the behavior you're
looking for, but in my opinion that's the most logical thing to happen.

I know this is not how UIAlertView behaves. I have no idea how Apple
handles this--I believe it may be the side-effect of a race condition (as
far as I can tell, they happen quite a bit when dealing with this) on their
end.

If that is not the behavior you want, I recommend using the
alertView:willDismissWithButtonIndex: delegate method. If you show the
new alert from within that method, the dimming view will not fade out and
the second alert will be presented more naturally.

Before I merge the changes into master, I'd like you to take a look at it.
Check out the queuing-issue branch (19436ddhttps://github.com/Scott90/SDCAlertView/commit/19436ddadc979510c6df5a43786ab7e85026dd6a),
and see if this fixes your problem (keeping in mind what I wrote above). If
all's well, I will merge the branch into master.

Reply to this email directly or view it on GitHubhttps://github.com//issues/25#issuecomment-39010048
.

[fisherds]

I'm working with ubragg.

We brought in the new code but we're having a new issue with the library. The dismissWithClickedButtonIndex:animated: call is never dismissing an alert view.

Tracing our issue we've found that the AlertViewCoordinators method enqueueDismissingAnimationOfAlert is called fine. Then the dismiss is not happening immediately, it's just being queued into the transitionQueue. Then the transitionQueue is never running. Seems like dismissAlertImmediately should have returned YES and the queue would never be used.

To be clear, clicking a button is fine, it is only when we dismiss programmatically via dismissWithClickedButtonIndex:animated:

Thoughts?

[sberrevoets]

Some things to note about the queue (just so we're on the same page):

• It's FIFO, so calls are handled in the order they come in

• Calls to show or dismissWithClickedButtonIndex:animated: are enqueued if an alert is, at the time of calling, being presented or dismissed.

• Immediate dismissal (through -[SDCAlertViewCoordinator dismissAlertImmediately:withButtonIndex:]) means: remove from the array and notify the delegate, don't mess with the UI.

  This is only possible if the alert being dismissed is not currently being presented (it has to wait for the presentation to finish) and if it's not currently visible (then we need to update the UI). If an alert satisfies both conditions (for example: you show alert1, then show alert2, and then dismiss alert1), it can be dismissed "immediately".

• At the end of a transition, if there is something in the queue, that transition will be handled. This will continue until the queue is empty.

the dismiss is not happening immediately, it's just being queued into the transitionQueue

Assuming the queue acts the way it should, that means some alert is currently in transition. In the code @ubragg has posted, an alert is shown on one line, and then immediately dismissed on the next. When the dismiss call comes in, that alert is still being shown and the dismiss call is enqueued. In my testing here, as soon as the showing finishes, the alert is dismissed again.

And yes, this is all using dismissWithClickedButtonIndex:animated:, not any user action like clicking the button or whatever.

If you're seeing something different, I'd love to see a sample project with this issue (preferable as isolated as possible).

[ubragg]

So it appears the issue we were having was related to the fact that we had an animating object in our alert view. Here is the most simple case I can demonstrate:

  SDCAlertView* alertView = [[SDCAlertView alloc] initWithTitle:@"Alert"
                                           message:nil
                                          delegate:nil
                                 cancelButtonTitle:nil
                                 otherButtonTitles:nil];
  UIActivityIndicatorView *spinner = [[UIActivityIndicatorView alloc]
                                      initWithActivityIndicatorStyle:UIActivityIndicatorViewStyleGray];
  [spinner startAnimating];
  [alertView.contentView addSubview:spinner];
  [alertView show];
  //[spinner stopAnimating];  // uncomment this line and it works again
  [alertView dismissWithClickedButtonIndex:0 animated:NO];

It turns out the fix in our case is pretty simple - we just have to be sure to explicitly stop the spinner animation before dismissing the alert view. Easy enough workaround for me. :)

Otherwise, your code seems to work great!

Thanks for the fast turnaround!

[sberrevoets]

I think I understand what's going on here. Below is a quick write-up in case you were wondering and for future reference.

Do the fixes solve your problem though? Can I merge this branch into master?

---

When a spinner (UIActivityIndicatorView) is added as a subview, it is sent the private method _didMoveFromWindow:toWindow:. If startAnimating was sent explicitly by the user before the spinner was added to the view hierarchy (which it keeps track of), _didMoveFromWindow:toWindow: calls startAnimating again.

startAnimating, in turn, uses Core Animation to animate the spinner. It uses addAnimation:forKey:, and that animation obviously stays around until you call stopAnimating.

In the most recent code, a new CATransaction is started when presenting an alert. As part of that transaction, the alert and spinner then get added to the view hierarchy. As per what I wrote above, that triggers a call to startAnimating, which adds an animation to the spinner's internals that doesn't go away until stopAnimating is called.

Because this animation keeps going until stopAnimating is called, the CATransaction completion block isn't called until then either. This completion block is used to control the internal state of SDCAlertViewCoordinator. Since it wasn't called, the coordinator still thinks the alert is still being presented, and so the next transition is never dequeued.

To verify my theory, I changed your code a little bit:

[spinner startAnimating];           
[alertView show];
//[spinner stopAnimating];  // uncomment this line and it works again
[alertView dismissWithClickedButtonIndex:0 animated:NO];
[spinner performSelector:@selector(stopAnimating) withObject:nil afterDelay:5];

You'll see that after 5 seconds stopAnimating is called, the animation finally finishes, the completion block gets called, the dismiss call is dequeued and the alert disappears.

As you have experienced, explicitly calling stopAnimating works around the issue. You could also call startAnimating on the spinner after sending the alert show, because then the animation is not considered part of the alert presenting CATransaction (that transaction is committed before show returns).

I hope what I was trying to explain made sense and that it wasn't just a bunch of gibberish. I'd be happy to explain it in more detail if needed.

[ubragg]

Thanks, Scott. Your explanation makes perfect sense, and your code works
great for the issue I was originally seeing. I support the plan to move it
in to master!

On Wed, Apr 2, 2014 at 8:07 PM, Scott Berrevoets
[email protected]:

I think I understand what's going on here. Below is a quick write-up in
case you were wondering and for future reference.

Do the fixes solve your problem though? Can I merge this branch into

master?

When a spinner (UIActivityIndicatorView) is added as a subview, it is
sent the private method _didMoveFromWindow:toWindow:. If this method was
called explicitly by the user before (which it keeps track of),
_didMoveFromWindow:toWindow: calls startAnimating again.

startAnimating, in turn, uses Core Animation to animate the spinner. It
uses addAnimation:forKey:, and that animation obviously stays around
until you call stopAnimating.

In the most recent code, a new CATransaction is started. As part of that
transaction, the alert and spinner then get added to the view hierarchy. As
per what I wrote above, that triggers a call to startAnimating, which
adds an animation to the spinner's internals that doesn't go away until
stopAnimating is called.

Because this animation keeps going until stopAnimating is called, the
CATransaction completion block isn't called until then either. This
completion block is used to control the internal state of
SDCAlertViewCoordinator. Since it wasn't called, the coordinator still
thinks the alert is still being presented, and so the next transition is
never dequeued.

To verify my theory, I changed your code a little bit:

[spinner startAnimating]; [alertView show];//[spinner stopAnimating]; // uncomment this line and it works again[alertView dismissWithClickedButtonIndex:0 animated:NO];[spinner performSelector:@selector(stopAnimating) withObject:nil afterDelay:5];

You'll see that after 5 seconds stopAnimating is called, the animation
finally finishes, the completion block gets called, the dismiss call is
dequeued and the alert disappears.

As you have experienced, explicitly calling stopAnimating works around
the issue. You could also call startAnimating on the spinner after
sending the alert show, because then the animation is not considered part
of the alert presenting CATransaction (that transaction is committed
before show returns).

I hope what I was trying to explain made sense and that it wasn't just a
bunch of gibberish. I'd be happy to explain it in more detail if needed.

Reply to this email directly or view it on GitHubhttps://github.com//issues/25#issuecomment-39408259
.

[ubragg]

Well, after lots more testing, I have actually had a couple cases where it seems the problem recurred, where I got into a state where there were multiple actions going on simultaneously and at some point an alert that was trying to show itself was prematurely dismissed and then no further alerts could become visible until I killed the app. Incidentally the background stayed dimmed, even though I could interact with it.

The problem is, I can't seem to reliably reproduce it. It seems the problem is at least rare now, and much better than before, but it seems the problem still exists at some level.

[sberrevoets]

It's very possible there are still issues. It's hard to come up with every possible scenario of showing/dismissing alerts and then code against that. That's why I implemented the queue this way: all animated calls are executed one after another, which to me seems the safest, but it's probably not perfect.

In any case, I think the behavior has improved, so unless you run into the problem again today and figure out how to replicate the small issues you saw earlier, I will merge this new code into master tonight.

Thanks to you both both for your help on getting this worked out!

[ubragg]

Hi Scott, I'm still running into issues here where my app gets into a bad state when with multiple alerts happening, which unfortunately seems to happen quite often in my app.

Fortunately, I've isolated the issue to an easily reproducible block of code that you can just drop into a viewDidAppear somewhere:

  SDCAlertView* alert1 = [[SDCAlertView alloc] initWithTitle:@"Alert 1" message:@"This alert appears."
                                           delegate:nil cancelButtonTitle:@"Cancel" otherButtonTitles:nil];
  [alert1 show];
  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
    NSLog(@"Dismissing first alert.");
    [alert1 dismissWithClickedButtonIndex:0 animated:YES];
  });

  SDCAlertView* alert2 = [[SDCAlertView alloc] initWithTitle:@"Alert 2" message:@"This alert appears briefly or not at all."
                                                    delegate:nil cancelButtonTitle:@"Cancel" otherButtonTitles:nil];
  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
    NSLog(@"Trying to show second alert.");
    [alert2 show];
  });

// If you include this code, it seems to fix the bad state, and then the third alert does show properly.
//  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1.5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
//    NSLog(@"Trying to dismiss second alert.");
//    [alert2 dismissWithClickedButtonIndex:0 animated:YES];
//  });

  SDCAlertView* alert3 = [[SDCAlertView alloc] initWithTitle:@"Alert 3" message:@"This alert never appears."
                                                    delegate:nil cancelButtonTitle:@"Cancel" otherButtonTitles:nil];
  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(3.0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
    NSLog(@"Trying to show third alert.");
    [alert3 show];
  });

Basically, if an alertview is dismissing with animation within 0.5 seconds or so of another one starting to show, both alertviews are hidden, as are all subsequent alertviews that might possibly be shown. At this point, the only recourse is to kill the app to be able to show any more alertviews.

Let me know if you think you might have any ideas how to fix this.

[sberrevoets]

That was just something I overlooked, though it looked a bit worse than it turned out to be. Check out 246acbb and see if that fixes things.

[ubragg]

You are the man! Thanks for the quick fix. Where is the tip jar?

[sberrevoets]

Hey, no problem, glad it's fixed! No tips necessary, just spread the word 😄

[ubragg]

Will do. :)