Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory leak is not checked for luatest.server instance #349

Closed
nshy opened this issue Jan 12, 2024 · 2 comments · Fixed by #357
Closed

Memory leak is not checked for luatest.server instance #349

nshy opened this issue Jan 12, 2024 · 2 comments · Fixed by #357
Assignees
@ochaplashkin
Labels
feature A new functionality

Comments

@nshy
Copy link
Contributor

nshy commented Jan 12, 2024 •
edited
Loading

In case of memory leak sanitized program exit with non zero exit code. But we do not check the exit code for luatest.server instance on server:stop() or server:drop(). As a result test will pass. In order to test for memory leaks we should fail in this case.

Or may be we should fail if there is ERROR: LeakSanitizer: detected memory leaks in stderr output of the program.

Changes for demo:

diff --git a/asan/lsan.supp b/asan/lsan.supp
index 3432da22c..914b7cc2a 100644
--- a/asan/lsan.supp
+++ b/asan/lsan.supp
@@ -55,7 +55,7 @@ leak:tt_bitset_iterator_init
 
 # test: box-py/args.test.py
 # source: /lib/x86_64-linux-gnu/libc.so*
-leak:libc.so*
+# leak:libc.so*
 
 # test: box-tap/schema_mt.test.lua
 # source: src/lib/salad/mhash.h
diff --git a/extra/exports b/extra/exports
index 1dbfedc5c..0dfb2e431 100644
--- a/extra/exports
+++ b/extra/exports
@@ -678,4 +678,6 @@ luaM_sysprof_stop
 # favor to it.
 luaopen_misc
 
+tarantool_do_leak
+
 # }}} LuaJIT API
diff --git a/src/main.cc b/src/main.cc
index 42f3a68a2..3d4b565a4 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -737,6 +737,9 @@ print_help(FILE *stream)
 	fprintf(stream, help_msg, tarantool_version());
 }
 
+int tarantool_do_leak;
+static char *leaked_pointer;
+
 int
 main(int argc, char **argv)
 {
@@ -1108,6 +1111,11 @@ main(int argc, char **argv)
 	/* freeing resources */
 	free((void *)instance.name);
 	free((void *)instance.config);
+	if (tarantool_do_leak) {
+		leaked_pointer = (char *)malloc(10000);
+		leaked_pointer[0] = 'a';
+		leaked_pointer = NULL;
+	}
 	tarantool_free();
 	ERROR_INJECT(ERRINJ_MAIN_MAKE_FILE_ON_RETURN, do {
 		int fd = open("tt_exit_file.txt.inprogress",

Test for leak detection on simple run:

-- test.lua
local ffi = require('ffi')
ffi.cdef[[
    int tarantool_do_leak;
]]
ffi.C.tarantool_do_leak = 1
 [tarantool]$ ./build-asan-debug/src/tarantool test.lua

=================================================================
==1450109==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 10000 byte(s) in 1 object(s) allocated from:
    #0 0x55ac66db9489 in malloc (/home/shiny/dev/tarantool/build-asan-debug/src/tarantool+0x11d4489) (BuildId: 265ed52bc3d20d699bdbce56ab8fbd720ee399cf)
    #1 0x55ac66e0c005 in main /home/shiny/dev/tarantool/src/main.cc:1115:28
    #2 0x7fc95d445ccf in __libc_start_call_main /usr/src/debug/glibc/glibc/csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: 10000 byte(s) leaked in 1 allocation(s).
 [tarantool]$ echo $?
1

Test for leak detection in luatest:

-- test/app-luatest/exit_code_test.lua
local t = require('luatest')
local server = require('luatest.server')

local g = t.group()

g.test = function(cg)
    cg.server = server:new()
    cg.server:start()
    cg.server:exec(function()
        local ffi = require('ffi')
        ffi.cdef[[
            int tarantool_do_leak;
        ]]
        ffi.C.tarantool_do_leak = 1
        t.assert(true)
    end)
end
[tarantool]$ test/test-run.py --builddir ../build-asan-debug app-luatest/exit_code
<snip>
======================================================================================
WORKR TEST                                            PARAMS          RESULT
---------------------------------------------------------------------------------
[001] app-luatest/exit_code_test.lua                                  [ pass ]
---------------------------------------------------------------------------------
Top 10 tests by occupied memory (RSS, Mb):
*  102.1 app-luatest/exit_code_test.lua

(Tests quicker than 0.1 seconds may be missed.)

---------------------------------------------------------------------------------
Top 10 longest tests (seconds):
*   1.03 app-luatest/exit_code_test.lua
---------------------------------------------------------------------------------
Statistics:
* pass: 1
 [tarantool]$

There is similar issue for core = tarantool tests tarantool/test-run#416.
@nshy nshy mentioned this issue Jan 15, 2024
Open
@locker
Copy link
Member

locker commented Jan 15, 2024

Maybe related: #252

@ylobankov ylobankov added the feature A new functionality label Jan 17, 2024
@Serpentian Serpentian mentioned this issue Feb 14, 2024
Merged
@nshy
Copy link
Contributor Author

nshy commented Mar 12, 2024

ASAN build instructions:

CC=/usr/bin/clang CXX=/usr/bin/clang++ cmake -B build-asan-debug \
        -DCMAKE_BUILD_TYPE=Debug \
        -DENABLE_WERROR=ON \
        -DENABLE_ASAN=ON \
        -DENABLE_UB_SANITIZER=ON \
        -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION=OFF \
        -DFIBER_STACK_SIZE=1280Kb \
        -DENABLE_FUZZER=ON \
        -DTEST_BUILD=ON
make -C build-asan-debug -j

Source this from tarantool dir (because of $PWD below) before running ASAN tarantool:

export \
  ASAN=ON \
  LSAN_OPTIONS=suppressions=$PWD/asan/lsan.supp \
  ASAN_OPTIONS=debug=true:heap_profile=0:unmap_shadow_on_exit=1:detect_invalid_pointer_pairs=3:symbolize=1:detect_leaks=1:dump_instruction_bytes=1:print_suppressions=0

ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 13, 2024
Add check for any errors when the server stops
e614154 
Unexpected errors may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            // there is an error in `stderr` here
        end)
        g.server:exec(function()
           // server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() // `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `ERROR` substring. If it is found, an error will be caused (test
will be failed).

Closes tarantool#349
This was referenced Mar 13, 2024
Merged
Open
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 14, 2024
Add leak detection during server termination
2e3ec34 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            // there is an error in `stderr` here
        end)
        g.server:exec(function()
           // server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() // `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `LeakSanitizer` substring. If it's found, an error will be caused
(test will be failed).

Closes tarantool#349
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 14, 2024
Add leak detection during server termination
5c5f1f9 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            // there is an error in `stderr` here
        end)
        g.server:exec(function()
           // server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() // `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `LeakSanitizer` substring. If it's found, an error will be caused
(test will be failed).

Closes tarantool#349
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 14, 2024
Add leak detection during server termination
02684f0 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            // there is an error in `stderr` here
        end)
        g.server:exec(function()
           // server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() // `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `LeakSanitizer` substring. If it's found, an error will be caused
(test will be failed).

Closes tarantool#349
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 14, 2024
Add leak detection during server termination
1de9aa7 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            -- there is an error in `stderr` here
        end)
        g.server:exec(function()
           -- server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() -- `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `LeakSanitizer` substring. If it's found, an error will be caused
(test will be failed).

Closes tarantool#349
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 14, 2024
Add leak detection during server termination
9abeed6 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            -- there is an error in `stderr` here
        end)
        g.server:exec(function()
            -- server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() -- `test_foo` has been passed
    end)

We have added a check of the `stderr` from the server for the presence
of the `LeakSanitizer` substring. If it's found, an error will be caused
(test will be failed).

Closes tarantool#349
ochaplashkin pushed a commit to ochaplashkin/luatest that referenced this issue Mar 19, 2024
Add memory leak detection during server process execution
db20d97 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            -- there is an error from LeakSanitizer in `stderr` here
        end)
        g.server:exec(function()
            -- server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() -- `test_foo` has passed
    end)

We have added a check for the `LeakSanitizer` substring in `stderr` of
the server process. If it's found, an error will be thrown and test will
fail.

Closes tarantool#349
ylobankov pushed a commit that referenced this issue Mar 19, 2024
@ylobankov
Add memory leak detection during server process execution
e3ea6fc 
Memory leak detection may occur when working with the server:

    g.test_foo = function()
        ...
        g.server:exec(function()
            -- there is an error from LeakSanitizer in `stderr` here
        end)
        g.server:exec(function()
            -- server is alive
        end)
        t.assert(true)
    end)

    g.after_all(function()
        g.server:drop() -- `test_foo` has passed
    end)

We have added a check for the `LeakSanitizer` substring in `stderr` of
the server process. If it's found, an error will be thrown and test will
fail.

Closes #349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ochaplashkin ochaplashkin

Labels
feature A new functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add memory leak detection during server process execution ochaplashkin/luatest
4 participants
@locker @nshy @ylobankov @ochaplashkin