GNOME Bugzilla – Bug 761479
Deleting VMs does not work when Boxes is closed shortly after deletion
Last modified: 2016-09-20 08:15:55 UTC
I deleted a bunch of VMs with Boxes because I ran out of disk space. But the VMs remained under ~/.local/share/gnome-boxes/images. Next time I reopened Boxes, they reappeared.
I closed Boxes, reopened it, deleted all the VMs again, closed Boxes, reopened it, deleted all the VMs, four times; eventually it worked and the VMs stayed deleted.
(In reply to Michael Catanzaro from comment #1) > I closed Boxes, reopened it, deleted all the VMs again, closed Boxes, > reopened it, deleted all the VMs, four times; eventually it worked and the > VMs stayed deleted. Is it possible this is a symptom of bug#751696 ?
(In reply to Zeeshan Ali (Khattak) from comment #2) > (In reply to Michael Catanzaro from comment #1) > > I closed Boxes, reopened it, deleted all the VMs again, closed Boxes, > > reopened it, deleted all the VMs, four times; eventually it worked and the > > VMs stayed deleted. > > Is it possible this is a symptom of bug#751696 ? Although that sounds unlikely. Did you: * dismiss the "box X has been deleted" notification? * did you quit the UI immediately after dismissing the notification?
(In reply to Zeeshan Ali (Khattak) from comment #3) > Although that sounds unlikely. Did you: > > * dismiss the "box X has been deleted" notification? No. > * did you quit the UI immediately after dismissing the notification? Probably. Now I'm getting deja vu, did I report this before...? The answer is yes, in bug #755010. There are some other issues referenced in that bug as well, though.
*** Bug 755010 has been marked as a duplicate of this bug. ***
Thinking more about this, is it really the right thing to delete the VMs if user doesn't react to the 'Undo' notification? What if they change their mind and accidently quit Boxes?
(In reply to Zeeshan Ali (Khattak) from comment #6) > Thinking more about this, is it really the right thing to delete the VMs if > user doesn't react to the 'Undo' notification? What if they change their > mind and accidently quit Boxes? Also what if while selecting boxes to delete, they select one (or two) they didn't actually want to delete and then in the panic to 'Undo' in time, they manage to quite Boxes. I can imagine that happening to me at least. :)
The undo pattern with in-app notification is precisely for that "i didn't mean to do that" moment. I don't consider the "trash" pattern where you go back to thing you deleted a week ago particularly interesting.
(In reply to Jakub Steiner from comment #8) > The undo pattern with in-app notification is precisely for that "i didn't > mean to do that" moment. I don't consider the "trash" pattern where you go > back to thing you deleted a week ago particularly interesting. That is not what I was talking about at all. :) I was more concerned about two mistakes in a row, second one made in the panic of the first one. :)
I'm pretty sure that if you delete a couple boxes, fail to press Undo, then quit the app, the boxes should stay deleted. It's super confusing for them to reappear.
Fixed in master. I'll cherry-pick it for 3.20.1. If you could test it out a bit, that would be lovely. ---------- commit: 5dceb3091d2ea5fa1d6fea7f8b7b813937fa6e24 app: Minor formatting fix Line was going beyond 180 chars. commit: f88ac9ed8cf2c2cb0bcc7549550d471ac12cdc85 notification: CancelFunc -> DismissFunc Rename CancelFunc to DismissFunc as not is the new name more appropriate, the 'Cancel' give the complete opposite idea to what it actually does in some cases, e.g deletion notification. commit: 2a32fe6dbd51d7f17375f084960fc5f38032a308 notificationbar: cancel() -> dismiss_all() Just a more appropriate name for this method since it dismisses notifications. commit: 7cc0eacc1359afa4e3721d31d0910020d458d795 Add AsyncLauncher class Add a singleton that will be responsible for launching tasks in threads and then keep track of all launched threads so that at application exit, we can wait for all threads to finish. commit: 5f5508c66a39960c7b45f0998bccdc0fcb6021dd Make use of new AsyncLauncher API This, combined with previous patches, fixes the issue of boxes not actually getting deleted if user exits Boxes quickly after deleting the boxes. commit: ae337cc2fcd917b2907eea2085334e1e28eed987 util: Drop now redundant run_in_thread()
Created attachment 324800 [details] screencast of bug (In reply to Zeeshan Ali (Khattak) from comment #11) > Fixed in master. I'll cherry-pick it for 3.20.1. If you could test it out a > bit, that would be lovely. Still broken with today's git master. Screencast attached.
(In reply to Michael Catanzaro from comment #12) > Created attachment 324800 [details] > screencast of bug > > (In reply to Zeeshan Ali (Khattak) from comment #11) > > Fixed in master. I'll cherry-pick it for 3.20.1. If you could test it out a > > bit, that would be lovely. > > Still broken with today's git master. Screencast attached. Yeah, i can reproduce it here too. Really weird, I did test multiple times before pushing the patches.. :(
Ah, ok. Seems the fix is unreliable since I can't reproduce 100% of the times.
commit: 1375c8dfe3d2b0db821cc68b41c867cd0c947c7c app: Simply quit() on removal of main window Our shutdown() implementation needs to cleanup/shutdown all windows and it won't do that if we already remove the main window from the list of all the windows (that shutdown() iterates over) before launching the shutdown process. This fixes the issue of notifications not getting dismissed (and hence deletion of boxes not happning for deletion notifications) if user quits Boxes by pressing the 'x' button in the titlebar, instead of Ctrl+q shortcut.