Closed Bug 145377 Opened 22 years ago Closed 22 years ago

browser crashes after clicking BACK on this url

Categories

(Core Graveyard :: Plug-ins, defect, P2)

x86
Windows NT
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: shrir, Assigned: srgchrpv)

References

()

Details

(Whiteboard: [ADT1][PL RTM][acrobat][fix-trunk] custrtm- [verified-trunk])

Attachments

(4 files)

seen on mozilla 1.0.0 winNT
I see some kind of odd load/reload on 4.79 ...but no crash there

steps:
load url in mozilla 1.0.0 (0517)
it loads up blank..click BACK
CRASH

stack:

nsDocShell::GetRootScrollableView 
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 6054] 
nsDocShell::GetCurScrollPos 
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 3028] 
nsDocShell::ScrollIfAnchor 
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 5211] 
nsDocShell::InternalLoad 
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 4576] 
nsDocShell::LoadURI [d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, 
line 665] 
nsDocShell::LoadURI [d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, 
line 2391] 
XPTC_InvokeByIndex 
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp, 
line 106] 
XPCWrappedNative::CallMethod 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp, line 
2028] 
XPC_WN_CallMethod 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp, 
line 1267] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 790] 
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2744] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 806] 
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2744] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 806] 
fun_apply [d:\builds\seamonkey\mozilla\js\src\jsfun.c, line 1555] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 790] 
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2744] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 806] 
js_InternalInvoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 881] 
JS_CallFunctionValue [d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 3426] 
nsJSContext::CallEventHandler 
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 1019] 
nsJSEventListener::HandleEvent 
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp, line 182] 
nsXBLPrototypeHandler::ExecuteHandler 
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLPrototypeHandler.cpp, line 
448] 
DoKey [d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLKeyHandler.cpp, line 
108] 
nsXBLKeyHandler::KeyPress 
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLKeyHandler.cpp, line 124] 
nsEventListenerManager::HandleEvent 
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp, line 
1654] 
nsXULElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3461] 
nsXULElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3442] 
nsXULElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3442] 
nsGenericElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp, line 1644] 
nsHTMLInputElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLInputElement.cpp, 
line 1411] 
PresShell::HandleEventInternal 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6105] 
PresShell::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6028] 
nsViewManager::HandleEvent 
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 2030] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 306] 
nsViewManager::DispatchEvent 
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1887] 
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 83] 
nsWindow::DispatchEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 869] 
0x00adf010 
nsWindow::DispatchKeyEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 2660] 
nsWindow::OnChar [d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, 
line 2810] 
nsWindow::ProcessMessage 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3459] 
nsWindow::WindowProc 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 1131] 
USER32.dll + 0x1820 (0x77e71820) 
0x011c0001
Summary: browser crashes after clicking BACK on this url → [acrobat]browser crashes after clicking BACK on this url
Attached file stacktrace
giving to av for review
Assignee: beppe → av
Priority: -- → P2
Target Milestone: --- → mozilla1.0
IE cannot display the file, it goes to some sort of a loop, you can see that by
blinking busy cursor. With Mozilla I see different stacks on crash, some of them
indicating recursive function calls and obvious stack overflow.
Attached patch patch proposalSplinter Review
the crash happens on dereferencing null prt
using NS_ENSURE_SUCCESS(GetPresShell(getter_AddRefs(shell)), NS_ERROR_FAILURE);

when GetPresShell()  can return NS_OK & shell==0 is not the right thing do use
the patch is obvious
how I've  got the crash:
1. goto http://access.adobe.com/browser/netscape/namedest/despair.pdf#stupidity
2. delete #stupidity <ENTER>
3. click BACK.
With the patch I still crash with this stack trace (the same as without the
patch). In fact, I never saw the originally reported call stack, although I can
consistently reproduce the crash.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.0rc3) Gecko/20020520
hmm, nsPluginByteRangeStreamListener::OnStartRequest()
in this case its mine:( 
investigating...
Assignee: av → serge
The file looks a bit corrupt but still shows fine if saved locally.
oh, well, we cannot keep strong reference on nsPluginStreamListenerPeer in 
nsPluginByteRangeStreamListener::nsPluginByteRangeStreamListener(nsPluginStreamL
istenerPeer* 
aPluginStreamListenerPeer)
{
  NS_INIT_REFCNT();
  // don't addref
  mFinalPluginStreamListener = aPluginStreamListenerPeer;
}
due to asynchronous nature of  necko channels, 
if we fire several channel->AsyncOpen() from nsPluginStreamInfo::RequestRead() 
and then kill the page these channels could be still alive and call as with 
On[Start,DataA,Stop] but at that point
nsPluginByteRangeStreamListener.mFinalPluginStreamListener points to deleted 
object.
I'm going to add nsSupportsWeakReference to class nsPluginStreamListenerPeer
it allows us to handle delayed response on byte range requests from http
channel properly. 
Please review.
BTW: with this patch v1 (attachment 84553 [details] [diff] [review]) I'm  crashing with original reported 
stack trace, I have to apply the first patch (attachment 84463 [details] [diff] [review]) to prevent 
dereferencing null prt in nsDocShell::GetRootScrollableView()
So we have 2 bugs here, shell split it?
Whiteboard: [PL RTM]
Comment on attachment 84553 [details] [diff] [review]
patch v1, add nsSupportsWeakReference to class nsPluginStreamListenerPeer

I tried this patch on my branch build and it definitely fixes the crash I saw
without it. The pdf file itself is not displayed though, but this is probably a
different issue.
Comment on attachment 84553 [details] [diff] [review]
patch v1, add nsSupportsWeakReference to class nsPluginStreamListenerPeer

r=av
Attachment #84553 - Flags: review+
Keywords: nsbeta1+
Summary: [acrobat]browser crashes after clicking BACK on this url → browser crashes after clicking BACK on this url
Whiteboard: [PL RTM] → [ADT1][PL RTM][acrobat]
Comment on attachment 84553 [details] [diff] [review]
patch v1, add nsSupportsWeakReference to class nsPluginStreamListenerPeer

Just a small nit, would it be possible to use some less generic variable name
here:

+  nsPluginByteRangeStreamListener(nsIWeakReference* aWeakPtr);
Darin: can you please review this for us, lots of network stuff touched
Keywords: patch, review
Comment on attachment 84553 [details] [diff] [review]
patch v1, add nsSupportsWeakReference to class nsPluginStreamListenerPeer

>Index: nsPluginHostImpl.cpp

>+  nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(request));
>+  if (!httpChannel) {
>+    return NS_ERROR_FAILURE;
>+  }
>+     
>+  PRUint32 responseCode = 0;
>+  rv = httpChannel->GetResponseStatus(&responseCode);
>+  if (NS_FAILED(rv) || responseCode != 200) {
>+    return NS_ERROR_FAILURE;
>   }
> 
>   // if server cannot continue with byte range (206 status) and sending us whole object (200 status)
>   // reset this seekable stream & try serve it to plugin instance as a file

so this confuses me a little.  it looks like you are going to fail if you get
a 200 response, but what if you get a 206... is that really a failure?	is it
that we shouldn't be reaching this code if we're getting a 206?

otherwise, this patch seems fine to me.  sr=darin
Attachment #84553 - Flags: superreview+
yes, we do not suppose to reach this code on 206,
unless nsMultiMixedConv::OnStartRequest() failed by some reason, in which case 
we failed too.
Comment on attachment 84553 [details] [diff] [review]
patch v1, add nsSupportsWeakReference to class nsPluginStreamListenerPeer

this patch checked in on the trunk
mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp
new revision: 1.386; previous revision: 1.385
for crash in nsDocShell::GetRootScrollableView I've opend bug 147256.
marked this as fix-trunk.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [ADT1][PL RTM][acrobat] → [ADT1][PL RTM][acrobat][fix-trunk]
Whiteboard: [ADT1][PL RTM][acrobat][fix-trunk] → [ADT1][PL RTM][acrobat][fix-trunk] custrtm-
adding adt1.0.0, nominating for the branch
Keywords: patch, reviewadt1.0.0
shrir, could you verify this on the trunk?
oh no, this still crashes on trunk 0528 on NT. What I did was this :

1 load http://access.adobe.com/browser/netscape/namedest/despair.pdf#stupidity
2 wait for a long time (blank page appears after acrobat splash screen)...   
'connecting access.adobe.com' ...
3 clicked on STOP and removed '#stupidity' and hit ENTER

BOOM !
http://climate.netscape.com/reports/incidenttemplate.cfm?bbid=6796148

It's the same trace...reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Shrirang, the stack trace from TB 6796148 belongs to bug 147256, which I opened 
(comment #19) to fix the crash you are seeing. To test the fix for the patch 
(attachment 84553 [details] [diff] [review]) you have to access big enough pdf file over http://,
e.g.  http://www.gl.iit.edu/wadc/history/Roswell/Report/pt03a.pdf  
click as fast as you can on different bookmarks or thumbnails in acrobat window, 
that hit reload or back button.
w/o this check in (try branch build) you'll get a stack trace av mentioned 
in attachment 84499 [details]. 
Sorry for the mess.
Status: REOPENED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
oops, I was clearly sleeping while verifying this one.
Thanks for the clarification, Serge!
I tested with the new steps, could not reproduce a crash anymore. 
Whiteboard: [ADT1][PL RTM][acrobat][fix-trunk] custrtm- → [ADT1][PL RTM][acrobat][fix-trunk] custrtm- [verified-trunk]
adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
Drivers approval. 
Keywords: adt1.0.0adt1.0.1+
please checkin to the 1.0.1 branch ASAP. once there, remove the mozilla1.0.1+
keyword and add the fixed1.0.1 keyword.
Keywords: mozilla1.0.1+
Attachment #84553 - Flags: approval+
checked in MOZILLA_1_0_BRANCH
mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp,v  <--  nsPluginHostImpl.cpp
new revision: 1.372.2.15; previous revision: 1.372.2.14
verified on 1.0.0 brnch build 0603 that this works just fine.
Status: RESOLVED → VERIFIED
Keywords: verified1.0.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: