Venkman crashs on profiling after clearing profile data [@ _callHook]

NEW
Assigned to

Status

Other Applications
Venkman JS Debugger
9 years ago
4 years ago

People

(Reporter: Victor STINNER, Assigned: timeless)

Tracking

({crash})

1.8 Branch
Future
x86
Linux
crash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [timeless: need patch], crash signature)

Attachments

(1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.8 (like Gecko)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11) Gecko/2007120410 Ubuntu/7.10 (gutsy) Firefox/2.0.0.11

When I profile my application, Firefox crashs around profiling stuff (JSD): _callHook() in jsd_step.c:170 and jsd_step.c:174. I looks like Firefox reuse free'd memory.

Reproducible: Always

Steps to Reproduce:
1. Open Firefox
2. Open Javascript Debuger (Venkman)
3. Enable profiling
4. Surf on the WWW
5. Go back to Venkman and click on Profile > Clear Profile Data
6. Continue to surf on the WWW
7. If Firefox doesn't crash, go back to step 5

Actual Results:  
gdb backtrace: don't trust gdb, bug is somewhere else (see Valgrind data below):

Program received signal SIGSEGV, Segmentation fault.
(gdb) where
#0  0xb7ef913a in js_DropObjectMap (cx=0x94d9f48, map=0x0, obj=0xb2429f8)
    at jsobj.c:2287
#1  0xb7efb9f3 in js_FinalizeObject (cx=0x94d9f48, obj=0xb2429f8)
    at jsobj.c:2753
#2  0xb7edd727 in js_GC (cx=0x94d9f48, gckind=GC_NORMAL) at jsgc.c:3028
#3  0xb7eb2a22 in JS_GC (cx=0x94d9f48) at jsapi.c:1873
(...)

-------------

Valgrind result: first Valgrind related to Javascript.

Invalid read of size 4
   at 0x78E71DD: _callHook (jsd_step.c:170)
   by 0x78E7372: jsd_FunctionCallHook (jsd_step.c:285)
   by 0x40667ED: js_Invoke (jsinterp.c:1359)
   by 0x59EC266: nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) (xpcwrappedjsclass.cpp:1453)
   by 0x59E5EB2: nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) (xpcwrappedjs.cpp:467)
   by 0x4161592: PrepareAndDispatch (xptcstubs_gcc_x86_unix.cpp:100)
   by 0x414B190: nsTimerImpl::Fire() (nsTimerImpl.cpp:397)
   by 0x414B927: handleTimerEvent(TimerEventType*) (nsTimerImpl.cpp:459)
   by 0x4146B46: PL_HandleEvent (plevent.c:688)
   by 0x4146E3A: PL_ProcessPendingEvents (plevent.c:623)
   by 0x4148FA7: nsEventQueueImpl::ProcessPendingEvents() (nsEventQueue.cpp:448)
   by 0x5A2F334: event_processor_callback(_GIOChannel*, GIOCondition, void*) (nsAppShell.cpp:67)
 Address 0x10B95E2C is 4 bytes inside a block of size 80 free'd
   at 0x402237F: free (vg_replace_malloc.c:233)
   by 0x78E54E8: jsd_ClearScriptProfileData (jsd_scpt.c:388)
   by 0x78E3A22: jsd_ClearAllProfileData (jsd_high.c:292)
   by 0x78E33CC: JSD_ClearAllProfileData (jsdebug.c:119)
   by 0x78E9BB8: jsdService::ClearProfileData() (jsd_xpc.cpp:2722)
   by 0x41609C8: XPTC_InvokeByIndex (in /usr/lib/firefox/libxpcom_core.so)
   by 0x59EE3A6: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (xpcwrappednative.cpp:2169)
   by 0x59F4C3E: XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) (xpcwrappednativejsops.cpp:1455)
   by 0x4066840: js_Invoke (jsinterp.c:1375)
   by 0x4068A10: js_Interpret (jsinterp.c:3944)
   by 0x4066CD3: js_Invoke (jsinterp.c:1394)
   by 0x4076B5C: js_InternalInvoke (jsinterp.c:1469)

And there is a similar bug on "_callHook (jsd_step.c:174)". The line 170 is "JSLL_SUB(ll_delta, now, callerpdata->lastCallStart);" and the line 174 is "JSLL_ADD(callerpdata->runningTime, callerpdata->runningTime, ll_delta);".

I think that callerpdata is the evil pointer to free'd memory.


Venkman version: 0.9.87.2
Firefox version: 2.0.0.11
OS: Ubuntu Gutsy
CPU: Pentium4, 32-bit, 3 GHz
Assignee: nobody → rginda
Component: Extension/Theme Manager → JavaScript Debugger
Product: Firefox → Other Applications
QA Contact: extension.manager → venkman
(Reporter)

Updated

9 years ago
Component: JavaScript Debugger → Extension/Theme Manager
Product: Other Applications → Firefox
Version: unspecified → 2.0 Branch
Component: Extension/Theme Manager → JavaScript Debugger
Product: Firefox → Other Applications
Version: 2.0 Branch → 1.8 Branch
Severity: normal → critical
Keywords: crash
Summary: Venkman crashs on profiling after clearing profile data → Venkman crashs on profiling after clearing profile data [@ _callHook]

Comment 1

9 years ago
So unfortunately, nothing is going to happen to this bug unless you can get me working steps to reproduce, preferably on trunk. I just don't have the time to randomly surf around waiting for my build to magically crash. I haven't seen this myself, and while I'd love to do something about it, I can't until you give me more information. Sorry!
Severity: critical → normal
(Reporter)

Comment 2

9 years ago
Ok, I tried to reproduce the crash on a fresh Firefox installation.

1- Start Firefox with a fresh (empty) profile
2- Install Venkman: https://addons.mozilla.org/en-US/firefox/addon/216
3- Restart Firefox to enable Venkman
4- Open Venkman (Tools > Javascript Debugger)
5- Enable Profiling (click on the big Clock icon, label Profile)
6- Go to a website using Javascript, eg. http://linuxfr.org/
7- In Venkman, clear profile data (Profile > Clear Profile Data)
8- Go to another page using Javascript, eg. http://linuxfr.org/journal/
9- Loop to step 7 until Firefox crashs

On my computer, it does crash at the step 8, sometimes I need to open more pages.

If it doesn't crash, go to Venkman and click on Profile > Display Profile Data, to use Javascript core functions.

The bug is an Heisenbug since it uses free'd memory. So it's hard to find an easy procedure to reproduce it...
(Reporter)

Comment 3

9 years ago
I tried to reproduce the bug on Windows XP SP2 with Firefox 2.0.11 and Vekman 0.9.87.2, but it never crashed. The bug is specific to Linux or it's harder to reproduce it on Windows.

So, bug workaround: use Windows instead of Linux ;-)
(Assignee)

Comment 4

9 years ago
jsd_ClearAllProfileData seems /relatively/ sound. it calls lock scripts.
 290 jsd_ClearAllProfileData(JSDContext* jsdc)
 294     JSD_LOCK_SCRIPTS(jsdc);
 298         jsd_ClearScriptProfileData(jsdc, current);
 302     JSD_UNLOCK_SCRIPTS(jsdc);

however, the code to get script profile data doesn't seem to call lock scripts.
 140         JSD_UNLOCK_SCRIPTS(jsdc);
 147                 pdata = jsd_GetScriptProfileData (jsdc, jsdscript);

 290 jsd_GetScriptProfileData(JSDContext* jsdc, JSDScript *script)
 292     if (!script->profileData)
 293         script->profileData = (JSDProfileData*)calloc(1, sizeof(JSDProfileData));
 295     return script->profileData;

i think this:
 147                 pdata = jsd_GetScriptProfileData (jsdc, jsdscript);

needs to be done while holding a lock.

while we're at it,

 238 JSD_ClearScriptProfileData(JSDContext* jsdc, JSDScript *script)
 241     jsd_ClearScriptProfileData(jsdc, script);

that's a public function which isn't holding a lock.

 116 JSD_ClearAllProfileData(JSDContext *jsdc)
 119     jsd_ClearAllProfileData(jsdc);    
this is the public function exercised by venkman (see below).

ClearProfileData
    * js/jsd/jsd_xpc.cpp, line 1360 (View change log or Blame annotations)
    * js/jsd/jsd_xpc.cpp, line 2740 (View change log or Blame annotations) 
Defined as a function prototype in:
    * js/jsd/idl/jsdIDebuggerService.idl, line 236 (View change log or Blame annotations)
    * js/jsd/idl/jsdIDebuggerService.idl, line 790 (View change log or Blame annotations) 
Referenced (in 2 files total) in:
    * js/jsd/jsd_xpc.cpp (View change log or Blame annotations)
          o line 1361
          o line 2741 
    * extensions/venkman/resources/content/venkman-commands.js (View change log or Blame annotations)
          o line 621
          o line 625
          o line 629 

summary:
_callHook needs to JSD_LOCK_SCRIPTS for its call to jsd_GetScriptProfileData
JSD_ClearScriptProfileData needs to JSD_LOCK_SCRIPTS for its call to jsd_ClearScriptProfileData.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

9 years ago
Created attachment 302321 [details] [diff] [review]
Possible patch

Like this?

I'll run a tryserver build with this patch and post URLs for the reporter to try when I have them.
Assignee: rginda → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #302321 - Flags: review?(timeless)

Comment 6

9 years ago
Egh, looks like the tryserver builds aren't going to work out until tomorrow night. Sorry! In the meantime you can build on your own, of course, if you feel up to it. :-)
(Assignee)

Comment 7

9 years ago
Comment on attachment 302321 [details] [diff] [review]
Possible patch

yeah, this is what i had in mind, but when i looked at the code, i found that it's not really sufficient to make jsd not broken. it might be sufficient to fix this crash.

I have a much bigger (yet unfortunately incomplete changeset) on my computer.
Attachment #302321 - Flags: review?(timeless) → review+

Comment 8

9 years ago
(In reply to comment #7)
> (From update of attachment 302321 [details] [diff] [review])
> yeah, this is what i had in mind, but when i looked at the code, i found that
> it's not really sufficient to make jsd not broken. it might be sufficient to
> fix this crash.
> 
> I have a much bigger (yet unfortunately incomplete changeset) on my computer.
> 

OK, so should I still check this in, or obsolete and wait for yours?

Comment 9

9 years ago
(In reply to comment #6)
> Egh, looks like the tryserver builds aren't going to work out until tomorrow
> night. Sorry! In the meantime you can build on your own, of course, if you feel
> up to it. :-)
> 

TryServer builds should now appear in a couple of hours in https://build.mozilla.org/tryserver-builds/?C=M;O=D (in a subdir which has my committer ID (gijskruitbosch@gmail.com) in it). I'd give a direct link, but they're not there yet and I'm going to catch some sleep, so. :-)

Comment 10

9 years ago
Reassigning per previous comments.
Assignee: gijskruitbosch+bugs → timeless
Status: ASSIGNED → NEW
Whiteboard: [firebug-p2]

Comment 11

9 years ago
I'm not sure what the state is here, but does not seem to involve Firebug, removing priority
Whiteboard: [firebug-p2] →
The state is that assigning to timeless without also nagging him on IRC every time he seems to be goofing off when he could be patching jsd is the same as not assigning.
Target Milestone: --- → Future
Whiteboard: → [timeless: need patch]
Crash Signature: [@ _callHook]

Updated

4 years ago
Attachment #302321 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.