The default bug view has changed. See this FAQ.

gtk2xtbin leaks client windows

RESOLVED FIXED in mozilla22

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

({mlk})

Trunk
mozilla22
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
xt_client_create creates a window for xtclient->child_widget through XtRealizeWidget().

xt_client_unrealize changes the window on the parent widget
xtclient->top_widget to old_window which is None.

When the parent widget is destroyed, the child_widget gets destroyed, but the
child_widget's window is not a descendant of the parent widget's window and so
the child widget's window does not get destroyed when the parent widget's
window is destroyed.
(Assignee)

Comment 1

6 years ago
When the window is destroyed during xt_client_unrealize, the acroread process started by the nppdf.so plugin spits out "unexpectedly destroyed" warnings and glib object type assertions and sometimes crashes.  Once the acroread process dies nppdf.so does not re-establish a connection, so the plugin fails to work.

These same errors already happen on shutdown while a plugin instance exists, but it would be more inconvenient to have the plugin fail while the application is still running.

XFixesChangeSaveSet can arrange for windows created by other clients to be automatically unmapped and reparented when the parent is destroyed.  This generates a BadMatch error when the child window has been created by the same client.
(Assignee)

Comment 2

4 years ago
Created attachment 696657 [details] [diff] [review]
destroy XtClient child window on unrealize

I still get warnings and assertions when destroying the child window, but I
haven't had an acroread crash yet with the latest version.

Other plugins such as vlc don't destroy their own windows but expect them to
be destroyed with the child window, leading to more windows leaked.

acroread is now not used by default for pdf documents (Preferences ->
Applications -> PDF needs to be changed to "Use Adobe Reader" "(in Nightly)").
Assignee: nobody → karlt
Status: NEW → ASSIGNED
Attachment #696657 - Flags: review?(stransky)

Comment 3

4 years ago
Comment on attachment 696657 [details] [diff] [review]
destroy XtClient child window on unrealize

Makes sense to me and I don't see any unexpected behaviour with this patch.
Attachment #696657 - Flags: review?(stransky) → review+
(Assignee)

Comment 4

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/54501cee764e
(Assignee)

Updated

4 years ago
Flags: in-testsuite-

Comment 5

4 years ago
I've just retested this patch (see Bug 814200 comment 26) and it causes a regression. nppdf.so crashes when more documents are open sequentially.
(Assignee)

Comment 6

4 years ago
I managed to reproduce with the build from Bug 814200 comment 26 by loading the same page again and again, but it took about a dozen attempts.  I ended up with a SIGSEGV in the acroread process at
XCreatePixmap (dpy=0x0, d=0, width=1, height=1, depth=195033592) at CrPixmap.c:50

acroread 8.5.1; gtk+ 2.20.1; libX11 1.4.0

That's a shame.  I guess I should back this out until bug 748923 is fixed.
Depends on: 748923
(Assignee)

Comment 7

4 years ago
backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04578917e3d5
Status: ASSIGNED → NEW
Whiteboard: [leave open]
(Assignee)

Comment 8

4 years ago
Firefox 19 uses pdf.js, so we don't need to support the acroread plugin now and I relanded.
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd103ec4c44b
Whiteboard: [leave open]
Sorry, I backed out this fix and several others on inbound because it looks like one of them broke B2G tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8c8d9373eba
Relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/caf862ef5cf2
https://hg.mozilla.org/mozilla-central/rev/caf862ef5cf2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.