nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.68k stars 3.13k forks source link

Memory leak in the socket #3439

Closed vidalouiswang closed 3 years ago

vidalouiswang commented 3 years ago

Expected behavior

Memory won't leak.

Actual behavior

Memory leak after client requested.

Test code

---------------------------- init.lua:
myhttp = {};

function myhttp.createServer(timeout)
    local n = net;
    local server = {server = n.createServer(n.TCP, (timeout or 28000))};
    timeout = nil;

    -- to create a new instance
    function server:new()
        local this = {};
        setmetatable(this, self);
        self.__index = self;
    end
    server:new();

    -- to create function to start to listen
    function server:listen(port, fn)
        -- default port is 80
        port = port or 80;
        self.server:listen(port, function(conn)
            port = nil;
            -- bind receive function
            conn:on("receive", function(sck, data)
                -- to match method, path and host information from raw data
                local _, _, method, url, host = data:find(
                                                    "(%u+)%s+(.+)%s+HTTP.+Host:%s*([%w%.]+).+");

                -- the following two tables will be send to callback as parameters
                local res, req = {
                    isHeaderWrote = nil, -- to mark if header has been sent to client(browser)
                    socket = sck, -- origin socket
                    headers = {}, -- headers to write
                    queue = {}, -- to store data to send
                    events = {}, -- to store events and its callback
                    finished = false -- to mark if need to close after write data to socket
                }, {
                    url = url, -- the path that browser request
                    method = method, -- the method 
                    host = host -- the host
                };

                -- to create a new instance of the response
                function res:new()
                    local this = {};
                    setmetatable(this, self);
                    self.__index = self;

                    -- a short name of self.socket:send
                    self.send = function(data)
                        self.socket:send(data);
                    end

                    -- bind sent callback for send next item in the queue 
                    self.socket:on("sent", function()
                        self:emit("drain"); -- emit the event to outer
                        if #self.queue > 0 then
                            self.send(table.remove(self.queue, 1)); -- send the item in the fifo
                            gc();
                        else
                            self:dispose(); -- release resources
                        end
                    end)
                end
                res:new();

                function res:dispose() -- release all resources
                    print(#self.queue, self.finished)
                    if #self.queue <= 0 and self.finished then
                        print("disposing")
                        -- release the callback of "sent"
                        self.socket:on("sent", nil);

                        -- self.socket:close will thore an error to raise esp8266 restart
                        -- so use pcall to execute
                        local function f()
                            self.socket:close();
                        end
                        pcall(f);

                        -- release the callback of "receive"
                        conn:on("receive", nil);

                        -- mark all tables as nil manually
                        -- f, res.socket = nil, nil;
                        -- res, req, conn = nil, nil, nil;
                        gc();
                    end
                end

                function res:on(event, fn) -- store the events and its callback
                    table.insert(self.events, {e = event, fn = fn});
                end

                function res:emit(event, data) -- find the event and execute its callback
                    if #self.events <= 0 then return; end
                    for _, value in pairs(self.events) do
                        if value.e == event then
                            value.fn(data);
                            break
                        end
                    end
                end

                function res:setStatusCode(code) -- set the status code for http
                    -- this function show be called before wirte
                    self.statusCode = code;
                end

                function res:write(data) -- write data to socket
                    if not self.isHeaderWrote then
                        -- write the http headers before body 
                        self.isHeaderWrote = true;

                        -- add all headers to the send queue 
                        for _, value in ipairs(self.headers) do
                            table.insert(self.queue, (value.name .. ": " ..
                                             value.value .. "\r\n"));
                        end
                        -- add endl
                        table.insert(self.queue, "\r\n");

                        gc();

                        -- send the "head" of the headers
                        local head =
                            "HTTP/1.1 " .. (self.statusCode or "200") ..
                                " OK\r\n" .. "Host: " .. host .. "\r\n";
                        self.send(head);
                    end

                    if data then -- enqueue the data
                        table.insert(self.queue, data);
                    end
                end

                function res:setHeader(name, value) -- set headers of the http
                    -- this function show be execute before the write
                    table.insert(self.headers, 1, {name = name, value = value});
                end

                function res:finish(data) -- to close the socket
                    self.finished = true; -- set the mark
                    self:write(data);
                    self:dispose(); -- release resources
                end
                -- execute the callback
                fn(req, res);
            end);
        end);
    end

    return server;
end

function configWifi(tb) -- to open an access point
    local w = wifi;
    tb = tb or {};

    -- set mode
    w.setmode(w.STATIONAP);

    -- open ap
    w.ap.config({
        ssid = (tb.apSsid or "test"),
        pwd = (tb.apPwd or "88888888"),
        auth = w.WPA_WPA2_PSK,
        save = false
    });

    -- set gateway and ip
    w.ap.setip({
        ip = "192.168.6.1",
        netmask = "255.255.255.0",
        gateway = "192.168.6.1"
    });
end

-- a short name
function gc() collectgarbage("collect"); end

-- open access point
configWifi({apSsid = "testAP", apPwd = "12345678"});

-- create server
local server = myhttp.createServer(3);
server:listen(80, function(req, res)
    -- set headers
    res:setHeader("Content-type", "text/html");
    res:setHeader("Connection", "close");

    -- route
    if req.url == "/" then
        -- require index.html
        local f = file.open("index.html", "r");

        local function sendIndex()
            gc();
            local d = f.read(128);
            if d then
                res:write(d);
            else
                f.close();
                res:finish();
                res:on("drain", nil);
                sendIndex = nil;
            end
        end

        -- bind the callback to send next chunk of the file
        res:on("drain", sendIndex)

        -- start to send
        sendIndex();
    else
        res:finish();
    end
end)

-- index.html:

<!DOCTYPE html>
<head>
    <title>888</title>
    <style>
        * {
            margin: 3px;
            padding: 6px;
            font-size: 1rem;
        }
        #mainDiv {
            padding: 6px;
            border: 1px solid black;
            position: absolute;
            left: 50%;
            top: 50%;
            transform: translate(-50%, -50%);
        }
    </style>
</head>

<body>
    <div id="mainDiv">
        <input id="filename" type="text" placeholder="File name"></input>
        </br><textarea id="code" placeholder="File content"></textarea>
        </br> <input id="upload" type="button" value="upload" />
        <input id="getfile" type="button" value="getfile" />
        <input id="run" type="button" value="run" />
        <input id="listfile" type="button" value="list file" />
        <input id="settime" type="button" value="update time" />
    </div>
    <script>
        let prefix = "oDAFS9ekz0UZC5AM";
        function get(id) {
            return document.getElementById(id);
        }
        let fileName = get("filename"),
            code = get("code");

        function post(path, content, callback) {
            var xhr = new XMLHttpRequest();
            xhr.open("POST", path, true);
            xhr.onreadystatechange = function() {
                if (xhr.readyState == 4 && (xhr.status == 200 || xhr.status == 304)) {
                    callback && callback(xhr.responseText);
                }
            };
            xhr.send(content);
        }
        get("upload").onclick = function() {
            post("/upload/" + fileName.value || "temp.lua",
            prefix + code.value || "");
        }
        get("getfile").onclick = function() {
            post("/getfile/" + fileName.value || "init.lua",
                "",
                function(e) {
                    code.value = e;
                });
        }
        get("run").onclick = function() {
            post("/run/tmp.lua", prefix + code.value || "")
        }
        get("listfile").onclick = function(){
            post("/listfile/tmp.lua", prefix+"",function(e){
                alert(e);
            })
        }
        get("settime").onclick =function(){
            let date = (parseInt(new Date().getTime()/1000)).toString();
            post("/settime/" + date+ "@" + "8888", prefix+"",function(e){
                alert(e);
            })
        }
    </script>
</body>

</html>

NodeMCU startup banner

NodeMCU 3.0.0.0 branch: release commit: d4ae3c364bd8ae3ded8b77d35745b7f07879f5f9 release: 3.0.0-release_20210201 +1 release DTS: 202105102018 SSL: false build type: float LFS: 0x40000 bytes total capacity modules: adc,coap,crypto,encoder,file,gpio,http,i2c,mdns,net,node,pwm,rtcfifo,rtcmem,rtctime,sjson,sntp,spi,tmr,uart,websocket,wifi build 2021-06-01 00:56 powered by Lua 5.3.5 on SDK 3.0.1-dev(fce080e)

Hardware

I'm using a typical NodeMCU 4MB dev board. I'm sure resources have been released after used, but every time after the client refreshed the page(to refetch the file "index.html") memory will be reduced by 1KB, until all memory gone the NodeMCU will be reboot.

vsky279 commented 3 years ago

Hi, this seems to be a bit complex for someone to get into it. Can you please provide a Minimal, Complete, and Verifiable example?

vidalouiswang commented 3 years ago

Hi, this seems to be a bit complex for someone to get into it. Can you please provide a Minimal, Complete, and Verifiable example?

Haha, I found an error in the process of streamlining the code. After the correction, the memory is no longer leaking. I had edited the code and thank you for your reply.

vsky279 commented 3 years ago

@vidalouiswang Good to hear that 👍. I think it is usually the case that the leak is likely in Lua code. This is why a minimal example is needed.