After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 684076 - clean up a11y on shutdown
clean up a11y on shutdown
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Accessibility
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 727172 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-09-15 10:44 UTC by William Jon McCann
Modified: 2014-07-19 20:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Shut down a11y when an app shuts down (3.07 KB, patch)
2012-09-15 10:45 UTC, William Jon McCann
committed Details | Review
Patch proposal (2.88 KB, patch)
2013-09-16 16:28 UTC, Mario Sánchez Prada
committed Details | Review

Description William Jon McCann 2012-09-15 10:44:57 UTC
It would be nice to clean up the a11y resources when they are no longer needed.

This helps clean up leak detections if nothing else.
Comment 1 William Jon McCann 2012-09-15 10:45:20 UTC
Created attachment 224390 [details] [review]
Shut down a11y when an app shuts down
Comment 2 Matthias Clasen 2012-09-15 19:13:22 UTC
Review of attachment 224390 [details] [review]:

Loks reasonably, though we should keep gtk_application_shutdown in sync with the exit code in gtk_main. Can you add the accessiblity shutdown call there too ? And maybe add a comment that points out that these blocks should stay in sync ?
Comment 3 Matthias Clasen 2012-09-16 23:37:33 UTC
Attachment 224390 [details] pushed as 5debed5 - Shut down a11y when an app shuts down
Comment 4 Mario Sánchez Prada 2013-09-16 15:46:29 UTC
I'm reopening this bug as per Benjamin's suggestion to fix an issue found and reported along with bug 708024.

You can see a description of it in [1] and the full IRC conversation in [2], but in a nutshell the problem is that this approach of calling shutdown() for the at-spi bridge when the topmost mainloop is being stopped and launched again several times from the same application, by means of different unnested calls to gtk_main(), that it's very specific but possible anyway.

So, I'm going to propose a revert of this patch for the patch committed along with this bug and attach it here for review.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=708024#c0
[2] https://bugzilla.gnome.org/show_bug.cgi?id=708024#c5
Comment 5 Mario Sánchez Prada 2013-09-16 15:47:00 UTC
*** Bug 708024 has been marked as a duplicate of this bug. ***
Comment 6 Mario Sánchez Prada 2013-09-16 16:28:13 UTC
Created attachment 255044 [details] [review]
Patch proposal

Patch undoing the old patch
Comment 7 Benjamin Otte (Company) 2013-09-16 16:31:32 UTC
Comment on attachment 255044 [details] [review]
Patch proposal

Looks good to me. As per API's comments on IRC, please only commit once 3.10 is out.
Comment 8 Mario Sánchez Prada 2013-09-16 16:43:32 UTC
(In reply to comment #7)
> (From update of attachment 255044 [details] [review])
> Looks good to me. As per API's comments on IRC, please only commit once 3.10 is
> out.

Ok. Thanks!
Comment 9 Mike Gorse 2014-03-27 17:19:37 UTC
Re-opening this since the patch was reverted for bug 708024. When removing the call to atk_bridge_adaptor_cleanup(), we missed that that function removes the sockets used for peer-to-peer D-Bus connections, so these files and their directories now do not get removed.
Comment 10 Mike Gorse 2014-03-27 17:22:57 UTC
*** Bug 727172 has been marked as a duplicate of this bug. ***
Comment 11 Mike Gorse 2014-04-10 04:04:45 UTC
I'm thinking that I should add an atexit handler to libatk-bridge to have it remove the socket and its directory if they exist, since it seems that there's no good place in gtk to call atk_bridge_adaptor_cleanup. Note that this won't remove the directory if the program terminates abnormally. Another option--although I don't think I want to do this for gnome-3-12--would be to signal at-spi2-registryd when a socket is created and have it remove the socket on a NameOwnerChanged signal, thus seeing that it will be removed even if the application crashes. I'm not sure off-hand if that will succeed if an application is running as root (although do we still have apps running as root these days?)
Comment 12 Mike Gorse 2014-04-22 14:21:05 UTC
There is not an atexit handler to remove the socket and its directory.

7e9fc5 (master)
238797 (gnome-3-12)

This doesn't remove the socket if the application terminates abnormally, so I'm going to leave this open for now.
Comment 13 Jan Alexander Steffens (heftig) 2014-06-28 19:28:09 UTC
The atexit handler in at-spi2-atk 2.12.1 does not work correctly. It does not extract the socket path from the DBus address, failing to unlink the socket:

unlink("unix:path=/run/user/1000/at-spi2-VS5THX/socket") = -1 ENOENT

So it still leaves around lots of socketdirs.
Comment 14 Mike Gorse 2014-07-19 20:11:52 UTC
Oops! Should be fixed now. Thanks for the report.

Master: 27cef1
gnome-3-12: 921bc0