422024 - crashes in free called by _releasevariantvalue

Status: RESOLVED FIXED

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

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One of the topcrashes in Fx3.0b4 is crashes in free called from http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/plugin/base/src/ns4xPlugin.cpp&rev=1.159&mark=1889#1889

These account for:
 * 3 of a sample of 10 of the crashes in RtlEnterCriticalSection (topcrash #2)
 * 1 of a sample of 10 of the crashes in free (topcrash #3)

The four that I looked at:
ea403114-ef1c-11dc-a707-001a4bd46e84
e4a84021-ef1c-11dc-92e3-001a4bd43ed6
d42e93e1-ef18-11dc-a301-001a4bd43e5c
4005345e-ef19-11dc-8538-001a4bd43ef6
all seem to be on Windows NT 5.1 service pack 2 (I think that's XP SP2), so it doesn't seem like the problem is bug 365986.

The common part of the stack is:

0  	free  	 jemalloc.c:5915
1 	_releasevariantvalue 	mozilla/modules/plugin/base/src/ns4xPlugin.cpp:1889
2 	CallNPMethodInternal 	mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1385
3 	CallNPMethod 	mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1394
4 	js_Invoke

Comment 1

[chris hofmann]

looks like this might be #4 top crash for beta5 

In particular the common part of stack mentioned in comment zero that looks like

0  	mozcrt19.dll  	free  	 jemalloc.c:6067
1 	xul.dll 	_releasevariantvalue 	mozilla/modules/plugin/base/src/ns4xPlugin.cpp:1889
2 	xul.dll 	CallNPMethodInternal 	mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1385
3 	xul.dll 	CallNPMethod 	mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1394
4 	js3250.dll 	js_Invoke 	mozilla/js/src/jsinterp.c:1287
5 	js3250.dll 	js_Interpret 	mozilla/js/src/jsinterp.c:4841
6 	js3250.dll 	js_Invoke 	mozilla/js/src/jsinterp.c:1303
7 	js3250.dll 	fun_apply 	mozilla/js/src/jsfun.c:1650
8 	js3250.dll 	js_Interpret 	mozilla/js/src/jsinterp.c:4824
9 	js3250.dll 	js_Invoke 	mozilla/js/src/jsinterp.c:1303
10 	js3250.dll 	js_InternalInvoke 	mozilla/js/src/jsinterp.c:1359


is seen a lot with comment where people are on yahoo mail...

> I am having consistent problems on Yahoo Webmail with these crashes for the last two betas.

> Yahoo! mail really doesn't work right... this is like the fifth time in two days!

> firefox 3 crashed when reading yahoo emails, especically a shipping notification email from newegg.com

Comment 3

[Stuart Parmenter]

this is the same really as the yahoo crash, but I'm going to put a patch in here.

Comment 4

[Stuart Parmenter]

Attachment: wip

we need something like this (untested)

Comment 5

[timeless]

sadly you shouldn't use #if for windows, just a bunch of getprocaddress calls, in order:

GetModuleHandleEx (you've written this code),
VirtualQuery

+#ifdef MOZ_MEMORY_WINDOWS
+          void *ptr = (void *)s->utf8characters;
HMODULE m = NULL;
if (GetModuleHandleEx)
+          GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, (LPCTSTR)ptr, &m);
else if (VirtualQuery) {
/* http://www.tech-archive.net/Archive/Development/microsoft.public.win32.programmer.kernel/2005-01/0167.html  http://www.microsoft.com/msj/archive/S1D4F.aspx  */
PMEMORY_BASIC_INFORMATION bi;
VirtualQuery((LPCVOID)ptr, &bi, sizeof bi);
m = (HMODULE)bi.BaseAddress;
}
if (m) {
+          CRTFREE = (CRTFREE)GetProcAddress(m, "free");
+          (CRTFREE)(ptr);

Comment 6

[Stuart Parmenter]

(In reply to comment #5)
> sadly you shouldn't use #if for windows, just a bunch of getprocaddress calls,
> in order:
> 
huh?

Comment 7

[Damon Sicore (:damons)]

Progress on this, Stuart?

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

14:08 < stuart> i'm going to miss triage -- my bug (422024) will be 
                fixed this afternoon

Comment 9

[Stuart Parmenter]

Attachment: fix

I tried all the various module related functions and none of them work properly.  This code isn't super cheap, but it should be called infrequently.

Comment 10

[Damon Sicore (:damons)]

So, who's supposed to review this?

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 318644 [details] [diff] [review]
fix

r+sr=jst, but I wonder if it'd be worth adding code to put a notification on the error console every time this happens so that plugin vendors have a chance to notice this and fix their code. W/o that, there's little incentive for plugin vendors to ever even know they're mis-using the API...