ReadyTalk / avian

[INACTIVE] Avian is a lightweight virtual machine and class library designed to provide a useful subset of Java's features, suitable for building self-contained applications.
https://readytalk.github.io/avian/
Other
1.22k stars 172 forks source link

Possible leak in compile function #447

Closed mretallack closed 9 years ago

mretallack commented 9 years ago

Hi,

I have been performing a valgrind check on the avian jvm and I think I may of found a leak in the compile.cpp - compile function.

It appears that stackMap is allocated and then passed to the Frame class, but is not free'ed. I have included a patch to show the changes that I have done. This appears to stop valgrind reporting the issue, however I do not feel that I understand the code enough to say that this is a problem and that the fix is correct.

Thanks Mark Retallack

Index: avian/src/compile.cpp
===================================================================
--- avian.orig/src/compile.cpp
+++ avian/src/compile.cpp
@@ -7030,9 +7030,9 @@ void compile(MyThread* t, Context* conte

             c->restoreState(state);

-            ir::Type* stackMap = (ir::Type*)malloc(
+            ir::Type* stackMap2 = (ir::Type*)malloc(
                 sizeof(ir::Type) * context->method->code()->maxStack());
-            Frame frame2(&frame, stackMap);
+            Frame frame2(&frame, stackMap2);

             unsigned end = duplicatedBaseIp + exceptionHandlerEnd(eh);
             if (exceptionHandlerIp(eh) >= static_cast<unsigned>(start)
@@ -7053,6 +7053,7 @@ void compile(MyThread* t, Context* conte
             context->eventLog.append(PopContextEvent);

             eventIndex = calculateFrameMaps(t, context, 0, eventIndex, 0);
+            free(stackMap2);
           }
         }
       }
joshuawarner32 commented 9 years ago

@mretallack Good catch, thanks!

FWIW, looks like it was my fault (replacing THREAD_RUNTIME_ARRAY with malloc because the former doesn't support non-POD types, forgetting the free): 7abbace8fb47f9a98f5323c7aaad21346bcde195

Another option would be to modify THREAD_RUNTIME_ARRAY to allocate it's memory as a char* and reinterpret_cast where necessary. That'll lose some type safety, though.

Your patch looks good. Do you want to make a pull request? Otherwise, I can commit it on your behalf.

joshuawarner32 commented 9 years ago

Also, I'm curious how you got avian to run correctly under valgrind? I'm not sure what changed, but it seems within the last year or so, avian stopped working under valgrind - it just crashes in initialization.

mretallack commented 9 years ago

Hi @joshuawarner32 , thanks for the info,

Its probable best if you apply the patch for now.

I am using the libjvm under our own application (using the JNI interface to start Avian) and I don't think I have had any problems running it under valgrind. Not sure if it makes any difference, but we are using i386 instead of x64.

Thanks again.