leaking memory if socket send failed? #194
Comments
|
This may be more of an hack than a solution. If the "sent" event handler doesn't run when it fails you could do something like this: conn:on("sent", function(conn)
delivered = true
end)
function send(data)
conn:send(data)
delivered = false
tmr.alarm(0, timeout, 0, function()
if delivered == false then
-- cleanup code
end
end)
end |
|
Thanks! It would help to release some memory. Right now, if not able to send, NodeCMU will just stuck and not release the memory used by socket. |
|
Sorry, the hack does not work. It will free the memory, but it would not able to send any more. |
|
Not being able to send after freeing up memory could be related to #202. |
|
Maybe there is possibility to have timeout configured for outbound sockets? |
|
I'm seeing a memory leak on connections whether they are successful or not. I'm not clear on all the conditions that lead to the leak, but the following code reproduces it. Note that this code isn't doing anything special at all; it just opens a connection, sends a request, and closes the connection when it receives a reply. I ran this a few times, then waited several minutes for the lazy garbage collection, but it made no difference; memory gets permanently eaten with each run of this function. Note that in this case, this happens even with successful send, so it is different than this issue's description, but it seems like it must be related. function TestSockLeak(ip, port)
collectgarbage()
print("start heap:", node.heap())
local c=net.createConnection(net.TCP, 0)
c:on("connection", function(conn)
print("event 'connection' happened, sending")
c:send("HEAD / HTTP/1.1\r\n\r\n")
end)
c:on("sent", function(conn)
print("event 'sent' happened")
end)
c:on("receive", function(conn, p)
print("event 'receive' happened, received: "..p:gsub("\r\n(.*)","...etc.",1))
conn:close()
end)
c:on("disconnection", function(conn)
print("event 'disconnection' happened")
collectgarbage()
print("collected garbage, heap now", node.heap())
end)
c:connect(port, ip)
end
> TestSockLeak("google.com",80)
start heap: 16152
> event 'connection' happened, sending
event 'sent' happened
event 'receive' happened, received: HTTP/1.1 200 OK...etc.
event 'disconnection' happened
collected garbage, heap now 15768
TestSockLeak("google.com",80)
start heap: 15592
> event 'connection' happened, sending
event 'sent' happened
event 'receive' happened, received: HTTP/1.1 200 OK...etc.
event 'disconnection' happened
collected garbage, heap now 15256
TestSockLeak("google.com",80)
start heap: 15280
> event 'connection' happened, sending
event 'sent' happened
event 'receive' happened, received: HTTP/1.1 200 OK...etc.
event 'disconnection' happened
collected garbage, heap now 14984
TestSockLeak("google.com",80)
start heap: 14848
> event 'connection' happened, sending
event 'sent' happened
event 'receive' happened, received: HTTP/1.1 200 OK...etc.
event 'disconnection' happened
collected garbage, heap now 14520
TestSockLeak("google.com",80)
start heap: 14512
> event 'connection' happened, sending
event 'sent' happened
event 'receive' happened, received: HTTP/1.1 200 OK...etc.
event 'disconnection' happened
collected garbage, heap now 14216 |
|
See #719 |
|
Actually, I'll re-open this one Terry. Using the TestSockLeak() function here I'm seeing a steadily declining heap even on 1.4. If I get some time I'll look into this, though it'd be great is someone else beats me to it ;) |
|
+1 I am assuming that a few of the issues will either need to stay open or be reopened, and a memory leak on core functionality is definitely a valid issue to be maintained. Sorry for missing this |
|
I can confirm this leaking still there though it is much better than dev120 |
|
Actually problem seems to be connected with referencing local variable c in 'connection' callback function. conn:send("HEAD / HTTP/1.1\r\n\r\n") |
|
FYI, my way of leakless GET (by trial and error, the point seems in releasing local conn = net.createConnection(net.TCP, 0)
conn:on("connection", function()
conn:send(("GET /home/pub?%s HTTP/1.0\r\n\r\n"):format(s), function()
conn:close()
conn = nil
collectgarbage()
end)
end)
conn:connect(80, "192.168.1.1") |
|
@dvv This is exactly the same problem, you are using local conn variable in the callback closure. local conn = net.createConnection(net.TCP, 0)
conn:on("connection", function(c)
c:send("GET / HTTP/1.1\r\n\r\n")
end)
conn:on("receive", function(c)
c:close()
end)
conn:connect(port, ip) |
|
It's not so much as |
|
@czarny My code is not meant to send like a machinegun. The code runs in a timer callback and it never leaked a byte for several months which gave me reason to state it as leakless. Other variations didn't. |
|
The example in the docs isn't very nice (and should have a "see also" to the HTTP module) but fortunately it doesn't make that mistake http://nodemcu.readthedocs.org/en/dev/en/modules/net/#example_5. |
|
Marcel, IMO, there's something wrong with the clean-up logic here. Apps programmers shouldn't be able to create this sort of unsweepable loop in Lua, so I've added this to my list. |
|
@marcelstoer This example will cause problems too. |
|
@TerryE and @czarny now you confused me...
This or that example i.e. the one in the docs?
Again, "here" being...what? Are you saying the example in docs is a bad example in general? AFAICS it doesn't create an upvalue. What am I missing? |
|
I am talking about example in the net module documentation. sk = net.createConnection(net.TCP, 0)
sk:on("receive", function(sck, c) print(c) end )
sk:connect(80,"192.168.0.66")
sk:on("connection", function(sck,c)
-- Wait for connection before sending.
sk:send("GET / HTTP/1.1\r\nHost: 192.168.0.66\r\nConnection: keep-alive\r\nAccept: */*\r\n\r\n")
end)Callback registered for 'connection' event keeps reference to sk variable, and sk keeps reference to callback closure what creates a reference cycle. To resolve this problem callback closure should use sck variable what will break the cycle. |
|
Grr...thanks, I found the missing 'c'. |
|
Marcel, let me track down the bug before we update this one sample. Thanks. |
|
OK, I've got my head around this. It's definitely a bug. I'll post some test cases to explain the issue. The underlying reason is that there are design flaws in the In fact the penny only dropped because I decided to completely rework the net module, and in doing so realised the issue. The new version isn't even at the PR stage because it is a fundamental restructuring, but it is less than the size of the original Anyway, I'll post these examples to explain the issues once I've tested them on a current dev build. |
|
I'm trying to get my head around it too -- I don't see where the base reference is to the The reference cycle itself should not cause a problem as the GC is mark and sweep rather than reference count based. |
|
Phillip, have a look and play with this gist and variations thereof. Try doing a restart and then doing the test1 multiple times. Not that I swapped out from google to a local apache instance for my connections. You will need to do similar. Replace the last timer by tmr.alarm(0,10000,0, function() enum_reg_funcs() g1=nil end)
tmr.alarm(1,20000,0, function() enum_reg_funcs() g2=nil end)
tmr.alarm(2,30000,0, enum_reg_funcs)Now dofile it and let the GC clean up. Do the same but re dofile it after 8 secs, say, when the connections have been closed but the clean down hasn't happened. Or set g1 and g2 to nil at the same time. Uaaaghhh |
|
There are so many execution paths that steadily fill the registry. |
|
I was using |
|
Which version of the firmware are you using? Various fixes were made in the dev branch. |
|
I'm using nodemcu_float_0.9.6-dev_20150704.bin. I had the same problem when I send HTTP GET every 10 seconds, on each connection about 700 bytes of heap was consumed. I solved the problem, open only one connection (with Keep Alive) and inside timer I just send data. Here is the code: SERVER_IP="127.0.0.1"
SERVER_PORT=80
CONST_CONN_SENT="sent"
CONST_CONN_RECEIVE="receive"
CONST_CONN_CONNECTION="connection"
HTTP_REQ_GET="GET /Service?id=NMCU001&data=XXXXX HTTP/1.1\r\nHost: www.somehost.com\r\nConnection: keep-alive\r\nAccept: */*\r\n\r\n"
-- SETUP ONLY ONE TCP CONNECTION AND CALL BACKS
local conn=net.createConnection(net.TCP, false)
conn:on(CONST_CONN_RECEIVE, function(conn, pl)
print(pl)
pl=nil;
collectgarbage();
end)
conn:on(CONST_CONN_SENT, function(conn) collectgarbage("collect") end)
-- CONNECT TO HOST
conn:connect(SERVER_PORT,SERVER_IP)
-- FINISH SETUP
-- FIRE TIMER EVERY 5 seconds TO SEND HTTP REQUEST
tmr.alarm(1, 5000, 1, function()
if (wifi.sta.getip() == nil) then
tmr.stop(1)
conn=nil;
collectgarbage();
else
print("timer:invoked()="..node.heap())
conn:send(HTTP_REQ_GET)
collectgarbage();
end |
|
@amancinellib, what happens on a very old release isn't relevant. |
|
I'm using build 0.96 dev, is a very old release ?
|
|
Oh yes, it is. You should build your own firmware, ideally from the |
|
Running the test code on the current |
|
Has been fixed in the meantime as per Arnim's comment -> closing. |
|
fixed in master Master 81ec366 |
I am running 3 ESP-12 modules, post sensors data to Internet every 60 seconds.
I noticed that if socket send failed, the heap size reduced by about 2K, if too many fails, it will cause out of memory panic and reboot.
Is there a way to listen to socket send fail event and release memory?
The text was updated successfully, but these errors were encountered: