LGTM

On Thu, Sep 11, 2008 at 3:36 PM,  <[EMAIL PROTECTED]> wrote:
> Reviewers: ager,
>
> Description:
> Fix issue 65 by making sure not to leak any of the cache
> tables when doing compilation cache operations.
>
> Please review this at http://codereview.chromium.org/1939
>
> Affected files:
>  M     src/compilation-cache.cc
>  M     test/cctest/test-api.cc
>
>
> Index: test/cctest/test-api.cc
> ===================================================================
> --- test/cctest/test-api.cc     (revision 276)
> +++ test/cctest/test-api.cc     (working copy)
> @@ -445,7 +445,7 @@
>  int TestAsciiResource::dispose_count = 0;
>
>
> -TEST(ScriptUsingStringResource) {
> +THREADED_TEST(ScriptUsingStringResource) {
>   TestResource::dispose_count = 0;
>   const char* c_source = "1 + 2 * 3";
>   uint16_t* two_byte_source = AsciiToTwoByteString(c_source);
> @@ -469,7 +469,7 @@
>  }
>
>
> -TEST(ScriptUsingAsciiStringResource) {
> +THREADED_TEST(ScriptUsingAsciiStringResource) {
>   TestAsciiResource::dispose_count = 0;
>   const char* c_source = "1 + 2 * 3";
>   {
> Index: src/compilation-cache.cc
> ===================================================================
> --- src/compilation-cache.cc    (revision 276)
> +++ src/compilation-cache.cc    (working copy)
> @@ -86,8 +86,14 @@
>
>  static Handle<JSFunction> Lookup(Handle<String> source,
>                                  CompilationCache::Entry entry) {
> -  Handle<CompilationCacheTable> table = GetTable(entry);
> -  Object* result = table->Lookup(*source);
> +  // Make sure not to leak the table into the surrounding handle
> +  // scope. Otherwise, we risk keeping old tables around even after
> +  // having cleared the cache.
> +  Object* result;
> +  { HandleScope scope;
> +    Handle<CompilationCacheTable> table = GetTable(entry);
> +    result = table->Lookup(*source);
> +  }
>   if (result->IsJSFunction()) {
>     return Handle<JSFunction>(JSFunction::cast(result));
>   } else {
> @@ -129,6 +135,7 @@
>  void CompilationCache::Associate(Handle<String> source,
>                                  Entry entry,
>                                  Handle<JSFunction> boilerplate) {
> +  HandleScope scope;
>   ASSERT(boilerplate->IsBoilerplate());
>   Handle<CompilationCacheTable> table = GetTable(entry);
>   CALL_HEAP_FUNCTION_VOID(table->Put(*source, *boilerplate));
>
>
>

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to