Memory leak at socket

[gbechtinger]

Expected behavior

No memory leak

Actual behavior

Memory leak at every call, goes from 39488 free heap to 28040 (within 20-30 calls) )and stabilizes, and the leak stop. An average of 200 bytes per call. i'm using the 2.0.0 sdk

Test code

Provide a Minimal, Complete, and Verifiable example which will reproduce the problem.

function httpGetter(getString)
        if ipfound ~= nil then
            print("ip is:"..ipfound.." and heap size is: "..node.heap())
            local r="GET "..getString.." HTTP/1.1\r\n\r\n"
            local conn = net.createConnection(net.TCP, 0)
            conn:on("connection", function(sck) sck:send(r) end)
            conn:on("sent",function(sck) print("Sucessfully sent") end)
            conn:on("receive", function(sck,c)
                print("Successfully received") 
                sck:close()
            end)
            conn:connect(80,ipfound)
        end
end

net.dns.resolve("www.dweet.io", function(dsck,ip)
    ipfound = ip
end)

tmr.alarm(1,2000,1,function() httpGetter("/mypage/page") end)

NodeMCU version

Dev master from frightanic

Hardware

NODEMCU LOL1N

[gbechtinger]

Solved. Adding the Connection:close header forces the server to disconnect after sending the reply and the leak is gone.

replace the line:
local r="GET "..getString.." HTTP/1.1\r\n\r\n"
with:
local r="GET "..getString.." HTTP/1.1\r\nConnection: close\r\n\r\n"

[devsaurus]

Thanks for posting the resolution, @gbechtinger.

@djphoenix is this some kind of FAQ which should be added to the net docs? Although the socket is closed from Lua side, the networking stack seems to need some support from peer side to quickly release heap.

[gbechtinger]

Just to clarify, using the connection:close header in this case, made the sck:close() useless,so I have removed it. I have to ignore the http module and use socket to the http request because http.get leaks memory when DNS failed to respond.

[devsaurus]

I have to ignore the http module and use socket to the http request because http.get leaks memory when DNS failed to respond.

Can't comment on a memory leak for failed DNS with http.get() and slightly off-topic, but have you also tried to http.get() from the resolved ipfound address? It would still be a workaround, but without low-level networking.

[gbechtinger]

No I didn't!

This is the final code, if someone needs:

srv = www.example.com (without http://)
getString = /mypage/etc/etc
func= callback function that will receive the HTTP reply content
It's also do a little DNS resolution backup to use if the DNS lookup fail. (In this case it will use the last ip returned by the DNS server)
Use:

myhttp("www.dweet.io","/get/latest/dweet/for/yourdweetname",function(data)
        if data ~= nil then print(data) end
end)

function myhttp(srv,getString,func)
    net.dns.resolve(srv, function(dsck,ip)
        if ip ~= nil then
            dns[srv]=ip   
        end
        local request = "GET http://"..srv..getString.." HTTP/1.1\r\nHost:"..wifi.sta.getip().."Content-Length:0\r\nConnection:close\r\n\r\n"
        local conn2 = net.createConnection(net.TCP, 0)
        conn2:on("connection", function(sck) sck:send(request) end)
        conn2:on("receive", function(sck,c) func(c:match("\r\n\r\n(.-)$")) end)
        conn2:connect(80,dns[srv])
    end)
end

[marcelstoer]

As for the perceived memory leak we've been through this a few times e.g. just 3d ago. The key phrase seems to be "...and stabilizes" which points to the lw IP TIME_WAIT setting. Of course I'm all for documenting that and the Connection: close workaround.

Also, it's true that the http.get is claimed to leak memory, #1721.

[coderkevin]

I'm also experiencing this using just http.post() to a local IP address URL. I get about 70 "HTTP client: Connection timeout" lines before the unit crashes and restarts. As long as the HTTP requests complete without errors, everything is fine though. 🤔

[felansu]

I have the same problem of @coderkevin

[gbechtinger]

@coderkevin @felansu AFAIK the only way to fix that is to forget the HTTP module and do http request direcly over NET module. I'm doing that!

[pjsg]

The GET request in @gbechtinger example above is rather odd -- the standard HTTP request has the verb (GET) followed by the path of the URL (i.e. not including the scheme or server). The Host: header contains the server name and not the client address......

[gbechtinger]

@pjsg @coderkevin @felansu @marcelstoer

This is the final code of my http request, I forgot to upload the final version. Please note that If it can't connect, the code retry the request:

I have notice that the Host header is ignored by some web servers that I use, but it needed when you acccess a server in a shared host enviroment like godaddy. That's why it worked with the client ip before.

function myhttp(srv,getString,func,recon)
    local sID = socketID
    socketID = socketID+1
    if recon then
        print("Reconnecting to: "..srv)
    end
    net.dns.resolve(srv, function(dsck,ip)
        if ip ~= nil then
          dns[srv]=ip   
        end
        local request = "GET http://"..srv..getString.." HTTP/1.1\r\nHost:"..srv.."\r\nContent-Length:0\r\nConnection:close\r\n\r\n"
        local conn2 = net.createConnection(net.TCP, 0)
        conn2:on("connection", function(sck) sck:send(request) end)
        conn2:on("receive", function(sck,c)
            if c~="0\r\n\r\n" then
                func(c:match("\r\n\r\n(.-)$"))
            end
        end)
        conn2:on("disconnection",function(sck,e) if e < 0 then myhttp(srv,getString,func,true) print("Socket disconnected") end end)
        conn2:connect(80,dns[srv])
    end)
end

how to use:

myhttp("www.site.com.br","/getCommand.php?username="..cfg[WEB_USERNAME].."&password="..cfg[WEB_PASSWORD],function(data)   
              --handle here the http reply as the data variable
        end)

[marcelstoer]

GET request [...] is rather odd

I noticed this as well but didn't really want to contribute to a closed issue. Surprised this even works...

It should be:

-- srv could be replaced with dns[srv]
local request = "GET "..getString.." HTTP/1.1\r\nHost:"..srv.."Content-Length:0\r\nConnection:close\r\n\r\n"

Also, I believe Content-Length:0 is superfluous. As GET requests have no request body the their size is 0 by definition, isn't it.

[coderkevin]

I noticed this as well but didn't really want to contribute to a closed issue.

Sounds like maybe this issue should be re-opened or another one opened in it's place?

[felansu]

I'm using 1.5, no errors, only bless !!

[marcelstoer]

I'm going to lock this conversation now before it meanders into even more directions.

As I said in my initial comment this wasn't really a leak. It's just that due to the TIME_WAIT configuration sockets were kept open longer than expected (and longer than necessary for most cases). The config value were adjusted in #1838.

Sending the Connection:close header with the request is a useful default behavior and that's now documented in our net.socket:on() example.

What Philipp and I last commented on was not the original closed issue but the oddities in the presented work-around code.

And last but not least the http.get leak is handled with #1721.