Bug 24615 - JavascriptCore leaks when creating and releasing context groups in a tight loop
Summary: JavascriptCore leaks when creating and releasing context groups in a tight loop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-16 06:37 PDT by Patrick Geiller
Modified: 2013-06-01 11:19 PDT (History)
6 users (show)

See Also:


Attachments
Test project reporting leaks when creating a global context, forcing GC, then destroying the context (52.75 KB, application/zip)
2009-03-16 12:26 PDT, Patrick Geiller
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Geiller 2009-03-16 06:37:26 PDT
JavascriptCore used in a Cocoa app reports leaks.

Just creating and destroying a context …
	JSGlobalContextRef ctx = JSGlobalContextCreate(NULL);
	JSGlobalContextRelease(ctx);

… will report this when terminating :
	LEAK: 71 Structure

When running the JSCocoa tests ( http://github.com/parmanoir/jscocoa/tree ) I get LEAK: 85 Structure. Is there a way to get a description of these objects ?
Comment 1 Mark Rowe (bdash) 2009-03-16 12:18:20 PDT
Do you still see the leaks if you force a garbage collection?
Comment 2 Patrick Geiller 2009-03-16 12:24:53 PDT
Yes, this code :
	JSGlobalContextRef ctx = JSGlobalContextCreate(NULL);
	JSGarbageCollect(ctx);
	JSGlobalContextRelease(ctx);

still reports the same leaks as without GC :
	LEAK: 71 Structure

And JSCocoa does GC before terminating, I've just checked.
Comment 3 Patrick Geiller 2009-03-16 12:26:51 PDT
Created attachment 28654 [details]
Test project reporting leaks when creating a global context, forcing GC, then destroying the context
Comment 4 Mark Rowe (bdash) 2009-03-16 12:47:09 PDT
You'd need to force the garbage collection after releasing the context, as the objects are presumably associated with the context and therefore not collected while the context is still live.
Comment 5 Patrick Geiller 2009-03-16 13:26:16 PDT
I've just tried that :
	JSGlobalContextRef ctx = JSGlobalContextCreate(NULL);
	JSGlobalContextRelease(ctx);
	JSGarbageCollect(ctx);

And leaks are down to 7. 

Doesn't JSGlobalContextRelease(ctx) render the context invalid ? I've seen that JSGarbageCollect(NULL) is now a no-op and calling it with a released context is counter intuitive.
Comment 6 Mark Rowe (bdash) 2009-03-16 13:30:49 PDT
You're right.  The new approach is for JSGlobalContextCreate to create a unique context group.  When the context is destroyed, the group should be destroyed too, and any memory allocated in it should be collected.  I wonder if the implicitly created context group is not being destroyed correctly.
Comment 7 Scott 2013-05-30 10:02:10 PDT
Four years later, I'm seeing this behavior in the JavaScriptCore framework installed on OS X 10.8.3.  Awesome.

Trivial experiments reveal that there's actually a leak of some sort in the JSContextGroupCreate / JSContextGroupRelease pair.   The following program consumes unbounded amounts of memory:


// testjsc.cpp.  To compile: clang++ testjsc.cpp -o testjsc -framework JavaScriptCore
#include <JavaScriptCore/JavaScript.h>
int main(int argc, char** argv) {
  while (true) {
    JSContextGroupRef contextGroup = JSContextGroupCreate();
    JSContextGroupRelease(contextGroup);
  }
  return 0;
}

For comparison, the following program never seems to consume more than about 4 MB:

#include <JavaScriptCore/JavaScript.h>
int main(int argc, char** argv) {
  JSContextGroupRef contextGroup = JSContextGroupCreate();
  while (true) {
    JSGlobalContextRef ctx = JSGlobalContextCreateInGroup(contextGroup, NULL);
    JSGlobalContextRelease(ctx);
  }
  JSContextGroupRelease(contextGroup);
  return 0;
}

So there's a leak in the context group code, but global contexts can be created and destroyed with no leaks as long as they reuse existing context groups.
Comment 8 Mark Hahnenberg 2013-05-31 15:19:23 PDT
I can't reproduce this issue with the JavaScriptCore.framework built from ToT WebKit. 

Scott, if it's possible could you please build ToT WebKit (or maybe download a nightly build and use the JavaScriptCore.framework that's inside of WebKit.app) and try building your test against that JavaScriptCore.framework? That would help a ton to see if the regression is indeed fixed in ToT.
Comment 9 Scott 2013-06-01 00:44:27 PDT
Mark --

I downloaded a Webkit nightly and supposedly built JavaScriptCore, but it's not clear to me how to make sure that an executable I build uses the new framework instead of the existing system framework in /System/Library/Frameworks.

In fact, the newly built JavaScriptCore framework itself seems to think it depends on the system's preinstalled JavaScriptCore, which I'm guessing is a problem:

>otool -L WebKit-r151049/WebKitBuild/Release/JavaScriptCore.framework/Versions/Current/JavaScriptCore
WebKit-r151049/WebKitBuild/Release/JavaScriptCore.framework/Versions/Current/JavaScriptCore:
	/System/Library/Frameworks/JavaScriptCore.framework/Versions/A/JavaScriptCore (compatibility version 1.0.0, current version 537.44.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
        [other libraries omitted]

Any ideas?  What exactly did you do to try to reproduce the issue?
Comment 10 Mark Rowe (bdash) 2013-06-01 01:07:12 PDT
Set the DYLD_FRAMEWORK_PATH environment variable to the path containing JavaScriptCore.framework before running your program (e.g., path/to/WebKit.app/Contents/Frameworks/10.8 if you're working with a nightly build and on OS X 10.8.x).
Comment 11 Scott 2013-06-01 01:50:35 PDT
(In reply to comment #10)
> Set the DYLD_FRAMEWORK_PATH environment variable to the path containing JavaScriptCore.framework before running your program (e.g., path/to/WebKit.app/Contents/Frameworks/10.8 if you're working with a nightly build and on OS X 10.8.x).

Yeah, I'd already tried that, but even with that variable set appropriately, running otools -L on my executable was still listing the old system framework as a dependency rather than the new one, so I wasn't sure it was actually going to do the right thing.  On the other hand, if I set DYLD_FRAMEWORK_PATH to the new framework's path, the leak does go away, so I suppose (1) otools is probably just lying to me about what will actually get used, and (2) the leak *has* actually been fixed.  Whew.  

A previous nightly I downloaded and compiled just a month or two, on the other hand, did leak memory on the same test at an absolutely furious pace...much faster than the preinstalled system framework leaks memory, even.  Is there any way to identify WebKit "releases" that are reasonably recent but also reasonably stable?  I've never even gotten a nightly to actually build WebKit all the way through...every time I've tried in the last month or two, including the one I downloaded just last night, the build has failed (specifically on NetscapePluginHostManager.mm, with "fatal error: 'WebKitSystemInterface.h' file not found
#import "WebKitSystemInterface.h"...pretty sure it's been the same build failure every time).

Anyhow, sorry for mostly wasting your time...thanks!
Comment 12 Mark Rowe (bdash) 2013-06-01 02:00:01 PDT
"otool -L" just dumps the Mach-O load commands in the given binary, it doesn't use any of dyld's logic to resolve those load commands to the actual library that will be loaded.

If you're interested in building WebKit, I'd suggest following the instructions at <http://www.webkit.org/building/build.html> and asking on the webkit-help mailing list if you run in to problems.
Comment 13 Mark Hahnenberg 2013-06-01 08:43:19 PDT
> A previous nightly I downloaded and compiled just a month or two, on the other hand, did leak memory on the same test at an absolutely furious pace...much faster than the preinstalled system framework leaks memory, even.  Is there any way to identify WebKit "releases" that are reasonably recent but also reasonably stable?  I've never even gotten a nightly to actually build WebKit all the way through...every time I've tried in the last month or two, including the one I downloaded just last night, the build has failed (specifically on NetscapePluginHostManager.mm, with "fatal error: 'WebKitSystemInterface.h' file not found
> #import "WebKitSystemInterface.h"...pretty sure it's been the same build failure every time).
> 
Hmm, I'm a little confused. Nightlies shouldn't have to be built/compiled. You just download them from http://nightly.webkit.org and the new JavaScriptCore.framework should be in /path/to/downloaded/WebKit.app/Contents/Frameworks/10.8/.

As for dealing with WebKitSystemInterface.h related build errors, http://www.webkit.org/building/build.html should help you with these issues as Mark Rowe mentioned above.

> Anyhow, sorry for mostly wasting your time...thanks!

Don't worry, it's not a waste of time to get the software working properly! If it still isn't working as expected, there might be some other issue that I'm not seeing.
Comment 14 Scott 2013-06-01 11:19:19 PDT
(In reply to comment #13)

> Hmm, I'm a little confused. Nightlies shouldn't have to be built/compiled. You just download them from http://nightly.webkit.org and the new JavaScriptCore.framework should be in /path/to/downloaded/WebKit.app/Contents/Frameworks/10.8/.

I'm investigating the feasibility of using JavaScriptCore as a general-purpose JavaScript engine inside my codebase without making a complete mess of my overall build system or making things too platform-specific, so I've been downloading source distributions and playing with those instead of prebuilt OS X-specific dmgs. (Partly for yuks, I wrote a script that agglomerates the JavaScriptCore source files into a single C++ file that compiles with minimal dependencies, although some of those "source files" are derived sources that were generated during the usual WebKit build process.  It's 12MB, takes about a minute to compile, and probably isn't particularly platform-independent yet, but so far it seems to compile and run on OS X without actually using any OS X frameworks.)

Since I'm thinking about using it in servers, memory leaks in JSC would be Bad.  I encountered at least two different memory leaks in the preinstalled system JSC framework right off the bat (one of them being the one exercised here), so I was about to give up on JSC and look into maybe using SpiderMonkey instead, but both of those leaks seem to be plugged in the nightly I just built, so I guess I'll play with JSC some more.
 
> As for dealing with WebKitSystemInterface.h related build errors, http://www.webkit.org/building/build.html should help you with these issues as Mark Rowe mentioned above.

Yeah, I'd hastily misread that page a while ago and not realized that build-webkit was in fact what I wanted for UNIX command-line builds...so I'd tried just running "make" in the top-level directory, and that managed to lull me into thinking it was the right thing to do since it at least got as far as building JSC correctly before crapping out on NetscapePluginHostManager.mm.  build-webkit does in fact seem to build the whole thing successfully, though.  (At least on my laptop.  It immediately fails on my desktop, but that's another story...looks like I've got an extra, incompatible version of QT or perl or something installed on the desktop.  Whee.  Well, I guess that's one thing an "All of JSC in one C++ file" agglomeration might be good for...)

Anyhow, thanks!