Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible window leak #9

Closed
UglyTroLL opened this issue Nov 17, 2014 · 8 comments
Closed

Possible window leak #9

UglyTroLL opened this issue Nov 17, 2014 · 8 comments

Comments

@UglyTroLL
Copy link
Contributor

Since the dismiss() method is doing some async work there(modal out animation, https://github.com/pedant/sweet-alert-dialog/blob/master/library/src/main/java/cn/pedant/SweetAlert/SweetAlertDialog.java#L310 ) , Dialog.dismiss() may not be invoked before the container activity actually stopped/finished.
Would be great if we can provide some sync dismiss() method there.

@pedant
Copy link
Owner

pedant commented Nov 18, 2014

when modal out animation ends, it will execute the super dismiss() https://github.com/pedant/sweet-alert-dialog/blob/master/library/src/main/java/cn/pedant/SweetAlert/SweetAlertDialog.java#L104 . When the dialog pops up, user must press "back" key, or click buttons that will call SweetAlertDialog.dismiss(). If you hide the dialog, remember to dismiss it when activity destroy.

I can't realize, under what situation does it cause a window leak?

@UglyTroLL
Copy link
Contributor Author

Yes I saw this code, but since it's executed in a different thread (asyncly), so if we do, e.g.:

sweetAlertDialog.dismiss();
parentActivity.finish();

Then it's most likely that the parent activity finishes before we actually dismiss(sweetAlertDialog.super.dismiss();) the dialog.
This scenerio is pretty common that, for example, if user clicks the 'Cancel' button on dialog, we finish the activity and go back to whereever the user was before.

@pedant
Copy link
Owner

pedant commented Nov 18, 2014

Hi, @UglyTroLL . I do a test in such a situation:

......    
.setConfirmClickListener(new SweetAlertDialog.OnSweetClickListener() {
                    @Override
                    public void onClick(SweetAlertDialog sDialog) {
                        sDialog.dismiss();
                        SampleActivity.this.finish();
                    }
                    })

then log at SweetAlertDialog dismiss, Modal out animation end, calling super.dismiss.
I got the console log following:

11-18 10:24:31.882    4728-4728/? D/sty﹕ call SweetAlertDialog dismiss()
11-18 10:24:32.101    4728-4728/? D/sty﹕ Modal out animation ended
11-18 10:24:32.203    4728-4728/? D/sty﹕ call dialog Super dismiss()

every time I clicked the button, I got the same output.
So the conclusion is, although the super dismiss will run asyncly, it can be still executed when parentActivity finish().

@UglyTroLL
Copy link
Contributor Author

Yes, but the sequence is not guaranteed, since it's, still, multi-threaded. That's why I put 'Possible' in the title, since I saw the 'window leak' error several times when I was testing on the real device.
My proposal is to add a dismissSync() method which dismiss the dialog right away (without the modal out animation). I'll send you a PR once I finished it (should be quick tho).

@pedant
Copy link
Owner

pedant commented Nov 18, 2014

Sorry about my fool. I have realized the problem.the better solution is super dismiss should not be overwrite, maybe implementing dismiss with a flag param. thank you, I will fix it soon!

@UglyTroLL
Copy link
Contributor Author

That would be great, thanks!

@pedant
Copy link
Owner

pedant commented Nov 27, 2014

overwrite cancel() 32edea2, so users press back-key or touch outside dialog that dismissWithAnimation() can be called.

@pedant pedant closed this as completed Nov 27, 2014
@UglyTroLL
Copy link
Contributor Author

Cool! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants