191675 - JS Shell (1.3.1+) Quit() calls exit() without cleaning up.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shanti Rao]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2) Gecko/20021126
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2) Gecko/20021126

Stack trace, from Borland C++:


Error 00002. 0x300010 (Thread 0xFFF87307):
Resource leak: The memory block (0x1A92198) was never freed

The memory block (0x01A92198) [size: 64 bytes] was allocated with malloc
Call Tree:
   0x00472E84(=JSDB.EXE:0x01:071E84) ..\js\jshash.c#65
   0x0047302A(=JSDB.EXE:0x01:07202A) ..\js\jshash.c#123
   0x0042C32F(=JSDB.EXE:0x01:02B32F) ..\js\jsstr.c#2817
   0x0042C409(=JSDB.EXE:0x01:02B409) ..\js\jsstr.c#2863
   0x004AA8F8(=JSDB.EXE:0x01:0A98F8) ..\js\jsapi.c#3607
   0x004099B1(=JSDB.EXE:0x01:0089B1) ..\rs\wrap_shell.cpp#246

------------------------------------------
Error 00004. 0x300010 (Thread 0xFFF87307):
Resource leak: The memory block (0x1A92174) was never freed

The memory block (0x01A92174) [size: 32 bytes] was allocated with malloc
Call Tree:
   0x00472E84(=JSDB.EXE:0x01:071E84) ..\js\jshash.c#65
   0x00472FE5(=JSDB.EXE:0x01:071FE5) ..\js\jshash.c#110
   0x0042C32F(=JSDB.EXE:0x01:02B32F) ..\js\jsstr.c#2817
   0x0042C409(=JSDB.EXE:0x01:02B409) ..\js\jsstr.c#2863
   0x004AA8F8(=JSDB.EXE:0x01:0A98F8) ..\js\jsapi.c#3607
   0x004099B1(=JSDB.EXE:0x01:0089B1) ..\rs\wrap_shell.cpp#246



Reproducible: Always

Steps to Reproduce:
run JS shell, and type print("hello").
print() calls JS_GetStringBytes().

Comment 1

[Phil Schwartau]

Reassigning -

Comment 2

[timeless]

Attachment: valgrind baseline: run js, quit()

ok, i ran valgrind 3 times on boffo (mozilla Qt tinderbox, running tip updating
constantly)

Comment 3

[timeless]

Attachment: valgrind leakline: run js, print("hello")\n quit()

Comment 4

[timeless]

Attachment: valgrind patched js.c to call JS_free(cx, JS_GetStringBytes outpu): run js, print("hello")\n quit()

Comment 5

[timeless]

Attachment: valgrind patched js.c to call JS_free(cx, JS_GetStringBytes output): run js, print("hello")\n quit()

Comment 6

[timeless]

Attachment: valgrind gc: run js, print("hello")\n gc()\n quit()

I set breakpoints at JS_GC...
xpcshell manually calls JS_ClearScope/JS_GC xpcshell.cpp:936/937

jsshell doesn't call it at all.
forcing a gc() - as shown in this log - does the right thing.

Comment 7

[timeless]

well... hrm... I grabbed js1.5rc4, and js1.5b1 and js1.3.1 and they all leak the
same way.  I don't know for certain that JS_DestroyContext isn't supposed to
free the memory itself; I tried reading
http://www.mozilla.org/js/spidermonkey/apidoc/sparse-frameset.html for the
various functions and didn't get any indication.

anyway.

JS_DestroyContext=>js_DestroyContext=>{stuff}+js_ForceGC=>js_GC

i'll attach a patch to js.c to decrease leak noise, but I really really can't
figure out if it's correct because I can't find any documentation :-(~
indicating whether JS_DestroyContext is guaranteed to cleanup everything or
whether the consumer is required to call JS_GC.  My money says js_DestroyContext
is historically broken and that it's what should be fixed, but ...

Comment 8

[timeless]

ok. This is really simple. I'm silly.

js.c's quit function Quit calls exit(...) which means that it doesn't clean
anything up :).

Comment 9

[timeless]

Attachment: port xpcshell.cpp behavior

ok. I ported the behavior from xpcshell.cpp into js.c for js1.3.1, and i
reached 0 leaks (it's really nice to have valgrind tell you that you aren't
leaking at all). the changes required for trunk are actually fewer than for
js1.3.1, unfortunately there are two leaks left, but it's still a big
improvement.

Comment 10

[timeless]

Comment on attachment 113527 [details] [diff] [review]
port xpcshell.cpp behavior

i'll remove the //exit.

sorry about the hot potato, rginda is too busy, dbradley isn't a peer although
he is familiar w/ the code for xpcshell.

Comment 11

[timeless]

checked in

Comment 12

[Phil Schwartau]

Checkin Verified -