davidm / luacom

Microsoft Component Object Model (COM) binding for Lua
http://lua-users.org/wiki/LuaCom
Other
116 stars 51 forks source link

Memory leak when getting SAFEARRAY from a COM server #22

Closed 1linux closed 4 years ago

1linux commented 4 years ago

Luacom can handle SAFEARRAYs, even nested SAFEARRAYS. However after transforming correctly to a Lua table, the SAFEARRAY seems to stay in memory. With bigger SAFEARRAYs and consecutive calls, memory ist being filled up quickly to 2GB, which lets break luacom with a memory exception.

Example C++ Code for a COM server method, delivering a SAFEARRAY:

    static TCOMCriticalSection CS;
    LPSAFEARRAY STDMETHODCALLTYPE TFooBarImpl::GetArray(long Lines)
    {
      TCOMCriticalSection::Lock Lock(CS);
      SAFEARRAYBOUND sabdBounds[1] = { {Lines, 0} };
      LPSAFEARRAY lpsaArray = SafeArrayCreate(VT_BSTR, 1, sabdBounds);
      return lpsaArray;
    }

and usage with Lua is

local comsrv=luacom.CreateObject("ComSrv.FooBar")
while true do
  local t=comsrv:GetArray(888)
end

Even if there are no valid BSTRings allocated, memory is quicklick filling up. The same would happen with VT_I4 integer elements.

1linux commented 4 years ago

A quick test with another scripting language (PHP) shows non of the memory leaks occuring with luacom

$s = new COM("ComSrv.FooBar",null,65001);
$x = array();
while (true) {
    $x = $s->GetArray(1000);
}

So the problem really seems to be the internal handling of SAFEARRAYs of luacom.

moteus commented 4 years ago

Can it be reproduced using some microsoft interfaces?

1linux commented 4 years ago

Yes, I wrote a litte demo-comserver with C++ Builder.

Source-Code and binary dll can be found here: http://wikisend.com/download/465180/ComServerLua.zip SHA-1 hash: A0C45A97D9CA662E0938B7065DC02EFDCA9790AF

Just have to "regsvr32 ComSrv.dll", and ""regsvr32 /u ComSrv.dll" when done

moteus commented 4 years ago

I think I found problebs. I found 2 places where VariantClear should be called. Can you pleas checkout my test branch moteus/luacom/safearray_memory_leak

1linux commented 4 years ago

I tried with your test branch, but still failing. Memory eats up quickly and finally Lua breaks with

C:\>lua5.1 test\luacom_test.lua
Moteus Version
lua5.1: test\luacom_test.lua:10: Internal Error:(.\src\library\tLuaCOMTypeHandle
r.cpp,1367):
stack traceback:
        [C]: in function 'GetArray'
        test\luacom_test.lua:10: in main chunk
        [C]: at 0x00401c20

I tried both with stock Lua 5.1.4 and Luajit 2.0, but the same error occurs on each.

moteus commented 4 years ago

I tests it with the TestSafeArray

local luacom = require"luacom"
local test = luacom.CreateObject("TestSafeArray.Test")
while true do
    test:GetArray()
end

And memory keeps on the same level. Are you sure you use correct version of the library ?

moteus commented 4 years ago

Just try you server and seems there exists another memory leaks. I did not this at first place because I have only x64 Lua on my machine and your lib is x86. I install it and now I see memory leak.

1linux commented 4 years ago

Jeah, the example server is just a small hack to reproduce the problem with a big ERP system, but it does it´s job. M$ seems to have hidden some unintuitive details in the SAFEARRAY implementation. Just don´t get the point why PHP & others do not have this problem...

moteus commented 4 years ago

I made one more fix and it seems to work. Can you please test it? But I found that LuaCom unpack entire array to the Lua stack, so in your example it requires up to 895 slots (888 elements plus some temporary elements). Not sure it is a good way for the huge arrays. For reference Lua stack.

Edit Also, can you please test array of variant. I am not sure should we call VariantClear in case of we got variant from SafeArrayGetElement (if there memory leak try to remove this condition)

1linux commented 4 years ago

I tried these cases: SAFARRAY[1000] of empty V_BSTR -> works SAFARRAY[32767] of SAFEARRAY[2] of V_BSTR -> works

Awesome. You made a man cry. Hopefully this patch makes its way upstream.