You have many memory leaks in your code, examples:
1. DebuggerData::DebuggerData():debugger_access_(OS::CreateMutex()),
2. TopData::TopData():break_access_(OS::CreateMutex()),
3. ThreadManagerData::ThreadManagerData():mutex_(OS::CreateMutex()),

Mutex* OS::CreateMutex() {
  return new LinuxMutex();
}

You didn't release the allocated mutex objects.

In the original code of V8 these variables were static, and as of that they
haven't released them explicitly since the operating system will do so
when the process is terminated, but here in our case we have to release every
dynamic allocated memory explicitly.

Regards,
Bahaa

On 2010/01/13 13:14:10, Bahaa Naamneh wrote:
1. allocation.h:
   "static int allocation_disallowed_;"
   should be removed from  NativeAllocationChecker class.
                
2. heap.h:
   a new class named ExternalStringTable has been added which contains
   2 static data members:
     static List<Object*> new_space_strings_;
     static List<Object*> old_space_strings_;
                
3. spaces.h:
   "static RSetState rset_state_;" should be removed form Page class.
                
4. bootstrapper.cc:
   a new static variable:
     List<char*>* delete_these_arrays_on_tear_down_;
                
5. In AssemblerData, the data member spare_buffer_ should be release
    by calling "DeleteArray" and not "delete" since it is allocated
    by NewArray:
     ~AssemblerData() { DeleteArray(spare_buffer_); }
                
6. Where do you release the allocated dynamic memory in: ?
    Counters::Counters()
     : state_counters(new StatsCounter[state_counters_count]) {

    You should add a destructor for Counters class to deallocated
    the memory.

7. The order of the member variables in your added classes do not
    match with the order they are written in the initialisation list in
    the constructor,
    and that cause the compilation under gcc to fail. you should
    reorder your initialisation lists or add '-Wno-reorder' flag to gcc
    compilation flags.
        

8. Note: a call to V8::Dispose() should be called explicitly at the
    end if each thread that creates an V8 instance to deallocate all
    the used memory.


9. There are some memory-leaks in V8 code which we have to take care
    of:

http://groups.google.com/group/v8-users/browse_thread/thread/04347322e009198c#
    The one discussed in the link above is a minor memory leak indeed
    but with a heavily used threads application it might be a problem.
        
10. Valgrind reported the following memory leaks:
60 bytes in 5 blocks are definitely lost in loss record 3 of 17
    at 0x1B903EB2: operator new(unsigned) (vg_replace_malloc.c:133)
    by 0x81C4EC6: v8::internal::CodeGeneratorData::result_constants_list()
(codegen.cc:70)
    by 0x8153134: v8::internal::Result::ConstantList()
(register-allocator.cc:48)
    by 0x80778FA: v8::internal::Result::ClearConstantList()
(register-allocator.h:99)
by 0x80774B8: v8::internal::CompilationZoneScope::~CompilationZoneScope()
(compiler.h:127)
    by 0x8073879: v8::internal::ScanMakeFunction(bool, bool,
v8::internal::Compiler::ValidationState,
v8::internal::Handle<v8::internal::Script>,
v8::internal::Handle<v8::internal::Context>, v8::Extension*,
v8::internal::ScriptDataImpl*) (compiler.cc:313)
    by 0x8073D10:

v8::internal::Compiler::ScanCompile(v8::internal::Handle<v8::internal::String>,
v8::internal::Handle<v8::internal::Object>, int, int, v8::Extension*,
v8::internal::ScriptDataImpl*) (compiler.cc:409)
    by 0x804EDC7: v8::Script::ScanNew(v8::Handle<v8::String>,
v8::ScriptOrigin*,
v8::ScriptData*) (api.cc:1273)


60 bytes in 5 blocks are definitely lost in loss record 4 of 17
    at 0x1B903EB2: operator new(unsigned) (vg_replace_malloc.c:133)
    by 0x81C4F04:
v8::internal::CodeGeneratorData::frame_element_constants_list()
(codegen.cc:76)
    by 0x80965F4: v8::internal::FrameElement::ConstantList()
(frame-element.cc:40)
    by 0x807791A: v8::internal::FrameElement::ClearConstantList()
(frame-element.h:98)
by 0x80774B3: v8::internal::CompilationZoneScope::~CompilationZoneScope()
(compiler.h:126)
    by 0x8073879: v8::internal::ScanMakeFunction(bool, bool,
v8::internal::Compiler::ValidationState,
v8::internal::Handle<v8::internal::Script>,
v8::internal::Handle<v8::internal::Context>, v8::Extension*,
v8::internal::ScriptDataImpl*) (compiler.cc:313)
    by 0x8073D10:

v8::internal::Compiler::ScanCompile(v8::internal::Handle<v8::internal::String>,
v8::internal::Handle<v8::internal::Object>, int, int, v8::Extension*,
v8::internal::ScriptDataImpl*) (compiler.cc:409)
    by 0x804EDC7: v8::Script::ScanNew(v8::Handle<v8::String>,
v8::ScriptOrigin*,
v8::ScriptData*) (api.cc:1273)


120 bytes in 6 blocks are definitely lost in loss record 6 of 17
    at 0x1B903EB2: operator new(unsigned) (vg_replace_malloc.c:133)
    by 0x81514C4: v8::internal::OS::CreateSemaphore(int)
(platform-linux.cc:640)
by 0x807E229: v8::internal::DebuggerData::DebuggerData() (debug.cc:1804)
    by 0x81B652F: v8::internal::V8Context::V8Context()
(v8-global-context.cc:117)
    by 0x81B7050: __static_initialization_and_destruction_0(int, int)
(v8-global-context.cc:29)
    by 0x81B7118: _GLOBAL__I__ZN2v87tls_keyE (objects.h:1939)


300 bytes in 5 blocks are definitely lost in loss record 10 of 17
    at 0x1B903EB2: operator new(unsigned) (vg_replace_malloc.c:133)
    by 0x81F9A19: v8::internal::ExternalReferenceTable::instance()
(serialize.cc:158)
    by 0x81F67DD:
v8::internal::ExternalReferenceDecoder::ExternalReferenceDecoder()
(serialize.cc:560)
by 0x81F6E03: v8::internal::Deserializer::Deserialize() (serialize.cc:669)
    by 0x81B77D2: v8::internal::V8::Initialize(v8::internal::Deserializer*)
(v8.cc:107)
    by 0x819D3AB: v8::internal::Snapshot::Deserialize(unsigned char const*,
int)
(snapshot-common.cc:43)
    by 0x819D441: v8::internal::Snapshot::Initialize(char const*)
(snapshot-common.cc:56)
    by 0x80551E7: v8::V8::Initialize() (api.cc:2737)


504 bytes in 18 blocks are definitely lost in loss record 12 of 17
    at 0x1B903EB2: operator new(unsigned) (vg_replace_malloc.c:133)
    by 0x81512F4: v8::internal::OS::CreateMutex() (platform-linux.cc:570)
    by 0x81B840D: v8::internal::ThreadManagerData::ThreadManagerData()
(v8threads.cc:275)
    by 0x81B605F: v8::internal::V8Context::V8Context()
(v8-global-context.cc:117)
    by 0x81B7050: __static_initialization_and_destruction_0(int, int)
(v8-global-context.cc:29)


81904 bytes in 1 blocks are possibly lost in loss record 16 of 17
    at 0x1B903D38: malloc (vg_replace_malloc.c:131)
    by 0x81BD241: v8::internal::Malloced::New(unsigned) (allocation.cc:38)
    by 0x8081CA6: v8::internal::Malloced::operator new(unsigned)
(allocation.h:40)
    by 0x809B4E0: v8::internal::GlobalHandles::Pool::Pool()
(global-handles.cc:219)
    by 0x809B456:
v8::internal::GlobalHandlesPrivateData::GlobalHandlesPrivateData()
(global-handles.cc:450)
    by 0x809A426: v8::internal::GlobalHandlesData::GlobalHandlesData()
(global-handles.cc:450)
    by 0x81B61F9: v8::internal::V8Context::V8Context()
(v8-global-context.cc:117)
    by 0x81B7050: __static_initialization_and_destruction_0(int, int)
(v8-global-context.cc:29)


82180 bytes in 5 blocks are definitely lost in loss record 17 of 17
    at 0x1B903D38: malloc (vg_replace_malloc.c:131)
    by 0x81BD241: v8::internal::Malloced::New(unsigned) (allocation.cc:38)
    by 0x81BB33C: v8::internal::Segment::New(int) (zone.cc:70)
    by 0x81BB00D: v8::internal::Zone::NewExpand(int) (zone.cc:182)
    by 0x8077CE2: v8::internal::Zone::New(int) (zone-inl.h:48)
by 0x80E3D94: v8::internal::ZoneObject::operator new(unsigned) (zone.h:142)
    by 0x819A800:
v8::internal::Scope::NewUnresolved(v8::internal::Handle<v8::internal::String>,
bool) (scopes.cc:255)
    by 0x8144D7A: v8::internal::Parser::ParsePrimaryExpression(bool*)
(parser.cc:3193)


please take a look at them.



There are still some minor things which I do not remember and didn't write
them
down since I wasn't focused on reviewing your code, but I was using your code
as
a reference during changing V8's code into thread-safe code.



http://codereview.chromium.org/435003
-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to