Closed Bug 61287 Opened 24 years ago Closed 24 years ago

The browser crashes on signing out of hotmail and msn calendar

Categories

(Core :: Layout, defect, P1)

x86
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla0.8

People

(Reporter: nasiruddin.shaikh, Assigned: buster)

References

Details

(Keywords: crash, Whiteboard: suntrak-n6-highp)

Attachments

(4 files)

Netscape build : 11/16 Platform : Solaris2.8(Sparc) (1) Launch the browser (2) Login to your hotmail account. (3) Click on 'Calendar' link (on the top-right side ) in hotmail. (4) Register with the 'Calendar' if you have not already registered. (5) Now click on the 'sign out' link on the top-right side of the page, and wait for the browser response. Expected result: The browser shouldnot crash. Actual result: The browser crashes.
Priority: P3 → P1
Whiteboard: suntrak-n6-highp
This bug does not happen on windows platform.
Keywords: crash
I think the reason of the bug is the following: destructor for some instance of object "nsFrameImageLoader" is invoked earlier than this instance completes fulfilling of method "NotifyFrames". This occurs because in method "StopLoadImage" of object "nsHTMLImageLoader" the following macro is invoked: "NS_RELEASE(mImageLoader)". This macro doesn't concern about reference counter and can delete object instance though there exist reference on this instance. Suggested fix is to replace macro "NS_RELEASE(mImageLoader)" to "NS_IF_RELEASE(mImageLoader)".
Attached patch This is fix of the bug (deleted) — Splinter Review
reassign to layout to get an official review and approval.
Assignee: asa → clayton
Component: Browser-General → Layout
Keywords: patch, review
QA Contact: doronr → petersen
First of all I apology for my previous comment and previous attachment. In my first comment the reason of the bug is not described correctly and patch doesn't fix problem indeed. I hope now I have correct solution of this problem. The reason of the is the following: destructor for some instance of object "nsFrameImageLoader" is invoked earlier than this instance completes fulfilling of method "NotifyFrames". This occurs because in method "Stop" of class "nsPresContext" the following method is invoked: "loader->StopImageLoad(aStopChrome)". And this method is invoked without checking whether 'loader' is being notified at this moment or not. It means that loading of image must be stopped anyway because in other all situations such checking is done by invoking method "nsFrameImageLoader::SafeToDestroy". In method "StopImageLoad" of class "nsFrameImageLoader" there exists the following line: 'mImageRequest->RemoveObserver(this);' in method "RemoveObserver" of class "nsImageRequest" destructor of instance of class "nsFrameImageLoader" is invoked though method "NotifyFrames" of same instance is not completed yet. My suggestion is: in method "nsFrameImageLoader::StopImageLoad" to check up whether current instance is being notified. If NO then method acts by the usual way. If YES then to interrupt loading of image by invoking the method "Interrupt" of class "nsImageRequest".
This crashs for me under Linux Redhat too. Tested with Dec 19th build.
Here's the call stack from my crash on Linux: Call Stack: (Signature = il_image_complete_notify() 9eb073a3) il_image_complete_notify() il_image_complete() ImgDCallbk::ImgDCBHaveImageAll() process_buffered_gif_input_data() gif_delay_time_callback() timer_callback() nsTimerGtk::FireTimeout() process_timers() TimerCallbackFunc() libglib-1.2.so.0 + 0x1104d (0x4063204d) libglib-1.2.so.0 + 0x10186 (0x40631186) libglib-1.2.so.0 + 0x10751 (0x40631751) libglib-1.2.so.0 + 0x108f1 (0x406318f1) libgtk-1.2.so.0 + 0x8c5b9 (0x405595b9) nsAppShell::Run() nsAppShellService::Run() main1() main() libc.so.6 + 0x189cb (0x4023e9cb)
Blocks: 63475
I found out that my last fix (attachment id 20977) leads to memory leak. The new fix solves this problem. The reason of memory leak: instance of "nsFrameImageLoader" which is being notified and released simultaneously is not released at all (as result of fix with interrupting of "mImageRequest") . So my suggestion is to return "NS_ERROR_FAILURE" in case of simultaneosly notification and releasing. So current instance of "nsFrameImageLoader" will complete notification and the next invocation of method "Stop" of "nsPresContex" will destroy the instance of "nsFrameImageLoader"
Buster: there is a patch attached -- could you review it or reassign to the right person for review? Thanks! Sergey Lunegov: Sorry for the slow response to your patch, it is very much appreciated! We'll get to it as soon as possible. Testcase for verification purposes coming up...
Assignee: clayton → buster
...or not. I cannot successfully login to Hotmail on my Linux machine to even try to reproduce this (https form submission problem probably). ChrisP: Over to you for a testcase if we need one. I'm marking this donttest for now to get it off our radar (feel free to remove that keyword if you want it back on your radar!).
Keywords: donttest
I'll check this in soon. A reproducable test case that does not involve logging into some service would be nice.
Severity: major → critical
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.8
I'm unable to reproduce the problem because hotmail is broken in mozilla (bug 50235). But I'm willing to take the patch since it's just a null pointer check. pam, will you review? I'll sr, and check it in if you say it's ok. I'll add an assertion if mImageRequest is null.
hmmm, actually, it's not a null pointer check. without a stack trace, it's hard to tell if this is the right thing or not. and, it's not really in pam's area at all. darn. ok, I'm on it. if someone could provide a stack trace, that would be a huge help.
Buster: I have a few bug reports that may be related to this bug. Let me see if I can dredge up an URL that will work as a test. -p
Here's the call stack from the Windows crash: il_image_complete_notify [d:\builds\seamonkey\mozilla\modules\ libimg\src\if.cpp, line 329] il_image_complete [d:\builds\seamonkey\mozilla\modules\ libimg\src\if.cpp, line 1645] ImgDCallbk::ImgDCBHaveImageAll [d:\builds\seamonkey\mozilla\modules\ libimg\src\if.cpp, line 191] process_buffered_gif_input_data [d:\builds\seamonkey\mozilla\modules\ libimg\gifcom\gif.cpp, line 694] gif_delay_time_callback [d:\builds\seamonkey\mozilla\modules\ libimg\gifcom\gif.cpp, line 728] timer_callback [d:\builds\seamonkey\mozilla\gfx\src\ nsImageSystemServices.cpp, line 72] nsTimer::Fire [d:\builds\seamonkey\mozilla\widget\ timer\src\windows\nsTimer.cpp, line 196] nsTimerManager::FireNextReadyTimer [d:\builds\seamonkey\mozilla\widget\ timer\src\windows\nsTimerManager.cpp, line 117] FireTimeout [d:\builds\seamonkey\mozilla\widget\ timer\src\windows\nsTimer.cpp, line 91]
In bug#59674, there is a similar call stack on linux. The test url used is: http://acw.activate.net/streetfusion/amd/slides/preloader.htm
The above url crashes on NT with: nsFrameImageLoader::NotifyFrames(int 0x00000000) line 570 + 3 bytes nsFrameImageLoader::Notify(nsIImageRequest * 0x08076710, nsIImage * 0x08079450, nsImageNotification nsImageNotification_kImageComplete, int 0x00000000, int 0x00000000, void * 0x00000000) line 508 ns_observer_proc(void * 0x08073b70, long 0x00000007, void * 0x0012f93c, void * 0x08076710) line 113 XP_NotifyObservers(OpaqueObserverList * 0x08076640, long 0x00000007, void * 0x0012f93c) line 259 + 28 bytes il_image_complete_notify(il_container_struct * 0x080763e0) line 327 + 18 bytes il_image_complete(il_container_struct * 0x080763e0) line 1644 + 9 bytes ImgDCallbk::ImgDCBHaveImageAll(ImgDCallbk * const 0x08076240) line 189 + 12 bytes il_jpeg_complete(il_container_struct * 0x080763e0) line 1002 so this may not be a good test url for the bug. It gets much farther and crashes in the FrameImageLoader....
OS: Solaris → Linux
Hardware: Sun → PC
Changing the platform/OS to PC/Linux (maybe it should be All/All). Has anybody tried out the latest patch suggested by Sergey Lunegov? Does this fix the problem?
r=karnaze
sr=buster
*** Bug 64957 has been marked as a duplicate of this bug. ***
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Blocks: 60740
Marking verified in March 6 Linux build
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: