Closed
Bug 569775
Opened 15 years ago
Closed 14 years ago
Out of process plugins can accidentally release XColormaps due to gdk bug.
Categories
(Core :: IPC, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b2
People
(Reporter: mattwoodrow, Assigned: karlt)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
cjones
:
review+
|
Details | Diff | Splinter Review |
Prior to Gtk+ 2.12.10, releasing a GdkColormap will also release the underlying XColormap. Fixed in http://git.gnome.org/browse/gtk+/log/gdk/x11/gdkcolor-x11.c?id=GTK_2_12_10 Plugins running in a separate process use separate refcounts and can accidently release Colormap objects required by the main thread.
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
Attachment #448935 -
Attachment is obsolete: true
Attachment #449172 -
Flags: review?(karlt)
Assignee | ||
Updated•15 years ago
|
Attachment #449172 -
Flags: review?(karlt) → review+
Comment on attachment 449172 [details] [diff] [review] Fixed erroneous unref >+ if (gtk_check_version(2, 12, 10)) { >+ /* Current gtk version is older than the fix */ Doesn't that "if" mean that the current gtk version is >= 2.12.10?
Assignee | ||
Comment 4•15 years ago
|
||
(In reply to comment #3) No, gtk_check_version returns pointer to a string if the library is not compatible with code compiled against 2.12.10. A "!= NULL" might make things a little clearer.
Assignee | ||
Comment 5•15 years ago
|
||
Tryserver crashtests have: REFTEST INFO | Loading file:///builds/slave/tryserver-linux-opt-unittest-crashtest/build/reftest/tests/modules/plugin/test/crashtests/540114-1.html NPP_Destroy WARNING: gdk_x11_colormap_foreign_new: assertion `GDK_IS_VISUAL (visual)' failed: 'glib warning', file /builds/slave/tryserver-linux/build/toolkit/xre/nsSigHandlers.cpp, line 193 (<unknown>:3026): Gdk-CRITICAL **: gdk_x11_colormap_foreign_new: assertion `GDK_IS_VISUAL (visual)' failed ###!!! [Parent][RPCChannel] Error: Channel error: cannot send/recv REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/tryserver-linux-opt-unittest-crashtest/build/reftest/tests/modules/plugin/test/crashtests/540114-1.html | plugin should not crash item 1 Thread 0 (crashed) 0 libxul.so!mozilla::plugins::PluginInstanceChild::AnswerNPP_SetWindow [PluginInstanceChild.cpp:10e17bfa2d0a : 818 + 0x0] eip = 0x016e51ed esp = 0xbfaf1e20 ebp = 0xbfaf2028 ebx = 0x01b73450 esi = 0xbfaf2160 edi = 0x0000000d eax = 0x00000000 ecx = 0x00000000 edx = 0x00000000 efl = 0x00010282 Found by: given as instruction pointer in context I'm guessing that probably means we have another bug that we are passing an invalid visual to the plugin.
Assignee | ||
Comment 6•15 years ago
|
||
There is a check for VisualID == None here: http://hg.mozilla.org/mozilla-central/annotate/cac6db5caec8/dom/plugins/PluginInstanceChild.cpp#l756 Another check for None would avoid this crash.
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → karlt
OS: Mac OS X → Linux
Assignee | ||
Comment 7•14 years ago
|
||
Using g_object_set_data avoids having to access the private ref_count.
Attachment #449172 -
Attachment is obsolete: true
Attachment #455001 -
Flags: review?(jones.chris.g)
Updated•14 years ago
|
Attachment #455001 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 8•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/3c108dceb5bd
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3b2
You need to log in
before you can comment on or make changes to this bug.
Description
•