Closed Bug 356182 Opened 18 years ago Closed 18 years ago

Crash when resizing transparent window using xul:resizer [@ nsWindow::ResizeTransparencyBitmap]

Categories

(Core Graveyard :: GFX: Gtk, defect)

x86
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: skrulx, Assigned: skrulx)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(7 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.7) Gecko/20060921 Ubuntu/dapper-security Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.7) Gecko/20060921 Ubuntu/dapper-security Firefox/1.5.0.7

Using xulrunner built from the branch tag SONGBIRD_0_2A_20060830_BRANCH, I get a crash (usually something like "*** glibc detected *** free(): invalid next size (normal)") whenever I resize a transparent window using a xul:resizer.  This does not happen when using the normal resizers on the window frame.

Attached is a stack trace as well as a xul window that demonstrates the problem.

(note that this is a real pain on gnome as once the app crashes the pointer gets stuck in the "drag" state and I have to switch to a virtual console and manually kill the xulrunner process.  To make this easier, I started running the test case in vncserver)

Reproducible: Always

Steps to Reproduce:
1. Launch xulrunner with the attached xul file as the defaultChromeURI
2. Grab one of the red resizers and resize the window
3. Observe the crash
Attached file stack trace of crash
Attached file xul window test case
I forgot to mention that I think the UpdateMaskBits() static function in nsWindow.cpp is causing the memory corruption.  Adding a "return" to the top of this function (so it does not execute) causes the crash to go away.  However, I could not see anything obviously wrong with this function.
Is there more that I can provide then the first attachment? (https://bugzilla.mozilla.org/attachment.cgi?id=241835)
Severity: normal → critical
Keywords: crash, testcase
Summary: Crash @ nsWindow::ResizeTransparencyBitmap when resizing transparent window using xul:resizer → Crash when resizing transparent window using xul:resizer [@ nsWindow::ResizeTransparencyBitmap]
hum

How about a valgrind log? I'm greedy :-)
Ok, here is a valgrind log collected by simply running the test application with "xulrunner/xulrunner -g -d valgrind application.ini".  I chopped off the beginning of the log to only include what was output after the test window had opened.  The "Invalid read of size 1" errors would start showing up after I began to resize the window using the xul:resizer and would crash after a few moves of the mouse (it does not crash immediately).  Note that I don't get any valgrind output when resizing the window using the window manager's window frame.

Let me know if the full valgrind output is useful or if you want me to run valgrind with any particular options.
You're not getting any assertion failures, right?
It looks like not.

Add
  printf("ChangedMaskBits mask=%d,%d rect=%d,%d,%d,%d\n",
         aMaskWidth, aMaskHeight, aRect.x, aRect.y, aRect.width, aRect.height);
to the start of ChangedMaskBits and
  printf("Allocating mask %d,%d -> %d\n", mBounds.width, mBounds.height, size);
before
  mTransparencyBitmap = new gchar[size];
and
  printf("RE-Allocating mask %d,%d -> %d\n", aNewWidth, aNewHeight, newSize);
before
  gchar* newBits = new gchar[newSize];

and then do the valgrind thing again... Thanks!!!
Ok I added the printfs and ran it under valgrind twice.  This run I was resizing using the right resizer.  It crashed pretty quickly as you can see.
Here is the second log with printfs using the left resizer to resize the window.  This took a lot longer to crash but eventually did.

Note that there is something strange about resizing a window with a xul:resizer.  The window tends to drift a bit when you are resizing it and winds up in a different location from when you started, not just a different size.  This does not happen when resizing with the window manager's window frame.  I don't know if this is related to this bug or not, or could just be some bad math in the resizer impl.
Looks like the mBounds.width changed without resizing the transparency bitmap ... I'll need to look closer to figure out how that happened
okay I found the problem. Do you want to take a stab at patching it? Here's what you have to do... mBounds.width/height get set in nsCommonWidget::Resize before we call NativeResize. So when NativeResize calls ResizeTransparencyBitmap, we exit early because the new size is the same as the current size.

The most robust solution is to add new members mTransparencyBitmapWidth and mTransparencyBitmapHeight to nsWindow. Track the width and height used to create the transparency bitmap in there. ResizeTransparencyBitmap should use those values for the bitmap's old width and height, instead of mBounds.width/height. Make sense?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Here is a patch that I think accomplishes what you described.  It does stop the crashing, although valgrind is not 100% happy.  I'll attach the new valgrind log after this.
ResizeTransparencyBitmap uses mBounds.width/mBounds.height as the old size of the bitmap, which is wrong. It needs to use mTransparencyBitmapWidth/Height.
Attached patch Missed a few... (obsolete) — Splinter Review
Ok, I fixed the stuff I missed in ResizeTransparencyBitmap() and I also modified ApplyTransparencyBitmap() to use the transparency dimensions rather than the mBounds (is this correct?)

After these changes, I am still getting some valgrind output the first time I focus the window:

==4792== Syscall param writev(vector[...]) points to uninitialised byte(s)
==4792==    at 0x75EC8DB: writev (in /lib/libc-2.3.6.so)
==4792==    by 0x6B033FB: (within /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6B08256: _XSend (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6AF8849: XPutImage (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6AE05D6: XCreateBitmapFromData (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x5B35144: gdk_bitmap_create_from_data (in /usr/lib/libgdk-x11-2.0.so.0.800.20)
==4792==    by 0xC39DFCE: nsWindow::ApplyTransparencyBitmap() (nsWindow.cpp:2968)
==4792==    by 0xC39E035: nsWindow::NativeShow(int) (nsWindow.cpp:2759)
==4792==    by 0xC3AE8C3: nsCommonWidget::Show(int) (nsCommonWidget.cpp:262)
==4792==    by 0xE682A3C: nsXULWindow::SetVisibility(int) (nsXULWindow.cpp:771)
==4792==    by 0xE676705: nsChromeTreeOwner::SetVisibility(int) (nsChromeTreeOwner.cpp:431)
==4792==    by 0xF11E572: nsGlobalWindow::Focus() (nsGlobalWindow.cpp:3566)
==4792==  Address 0x11004255 is 101 bytes inside a block of size 12,688 alloc'd
==4792==    at 0x4A19A16: malloc (vg_replace_malloc.c:149)
==4792==    by 0x6B0918F: _XAllocScratch (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6AF8033: (within /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6AF8849: XPutImage (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x6AE05D6: XCreateBitmapFromData (in /usr/lib/libX11.so.6.2.0)
==4792==    by 0x5B35144: gdk_bitmap_create_from_data (in /usr/lib/libgdk-x11-2.0.so.0.800.20)
==4792==    by 0xC39DFCE: nsWindow::ApplyTransparencyBitmap() (nsWindow.cpp:2968)
==4792==    by 0xC39E035: nsWindow::NativeShow(int) (nsWindow.cpp:2759)
==4792==    by 0xC3AE8C3: nsCommonWidget::Show(int) (nsCommonWidget.cpp:262)
==4792==    by 0xE682A3C: nsXULWindow::SetVisibility(int) (nsXULWindow.cpp:771)
==4792==    by 0xE676705: nsChromeTreeOwner::SetVisibility(int) (nsChromeTreeOwner.cpp:431)
==4792==    by 0xF11E572: nsGlobalWindow::Focus() (nsGlobalWindow.cpp:3566)

This only happens the very first time I focus the window and does not happen with subsequent focuses.  I added some debug output to make sure that the transparency dimensions was the same as the mBounds when ApplyTransparencyBitmap() is called, and it is.

Also, there seems to be a repainting problem.  When I make the window wider (using either the native resizer or the xul:resizer) the newly exposed area is transparent.  If I force a repaint by putting some other window above the test window and then refocusing the test window, the transparent area goes away and gets painted properly.
Attachment #242095 - Attachment is obsolete: true
Not sure about that valgrind warning. It may be an internal xlib issue.

This patch looks good. The repaint issue is likely a separate problem. Does it make a difference how much you resize at once? Is the problem only if you resize in the X direction or does it also occur when you resize vertically? ResizeTransparencyBitmap might need to fill in the ends of rows to make them opaque...
Comment on attachment 242212 [details] [diff] [review]
Missed a few...

We should get this checked in. You could get someone to check it in for you from Mozilla IRC (irc.mozilla.org #developers)
Attachment #242212 - Flags: superreview+
Attachment #242212 - Flags: review+
Assignee: nobody → skrulx
Whiteboard: [checkin needed]
Thanks for walking me through this, Robert.  I'll try to get the patch committed today.

As for the repaint issue, it does not seem to matter how much I resize the window and it fails to repaint when I resize both horizontally and vertically.  I'll attach a screen shot.  In this case, the window should be entirely black with the red resizer handles on either side.

This was checked in by timeless earlier today.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Thanks Steve.

At the ends of the two nsWindow::NativeResize methods, try adding a call to ApplyTransparencyBitmap.

I tried this and it had the effect of fixing the repaint problem only when resizing the window using the xul:resizer and not the native window manager's resizer.

I tried adding it at the end of ResizeTransparencyBitmap(), which worked for both cases, but I don't know if it is optimal.  It also worked when I added it to OnSizeAllocate(), which might be a little better.
I'm surprised it worked at the end of ResizeTransparencyBitmap, because we haven't resized the native widget there (in some cases, anyway)

Let's go with OnSizeAllocate. I'll gladly review another patch :-)
Comment on attachment 242212 [details] [diff] [review]
Missed a few...

mozilla/widget/src/gtk2/nsWindow.cpp 	1.187
mozilla/widget/src/gtk2/nsWindow.h 	1.66
Attachment #242212 - Attachment is obsolete: true
i don't like resolving bugs when i don't know if they're fixed. if you want to reopen this, please do, otherwise please indicate where the new bug is for comment 25.
Product: Core → Core Graveyard
Crash Signature: [@ nsWindow::ResizeTransparencyBitmap]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: