g_test_build_filename and friends not safe to call after g_test_run() has finished
While trying to track down some issues in the libsoup test suite, I found that libsoup#175 might be a result of a use-after-free when running as an installed-test. The test suite starts Apache with a command that looks entirely reasonable:
# Apache command: '/usr/sbin/apache2' '-d' '/usr/libexec/installed-tests/libsoup-2.4' '-f' 'httpd.conf' '-c' 'ErrorLog /tmp/test-tmp-libsoup-2.4_pull-api-test.test-2XPOE1/error.log' '-c' 'PidFile /tmp/test-tmp-libsoup-2.4_pull-api-test.test-2XPOE1/httpd.pid' '-k' 'start'
but then when it comes to shut it down after g_test_run()
has returned, it is accessing what looks like a freed string:
# Apache command: '/usr/sbin/apache2' '-d' '/tmp/test-tmp-libsoup-2.4_pull-api-test.test-2XPOE1/`{³rU' '-f' 'httpd.conf' '-c' 'ErrorLog /tmp/test-tmp-libsoup-2.4_pull-api-test.test-2XPOE1/error.log' '-c' 'PidFile /tmp/test-tmp-libsoup-2.4_pull-api-test.test-2XPOE1/httpd.pid' '-k' 'graceful-stop'
(Notice the argument to -d
.)
I think this is either a problem with g_test_build_filename()
and g_test_get_dir()
, or a problem with how the libsoup test suite is using them. They use static global variables test_disted_files_dir
and test_built_files_dir
, which are initialized by g_test_init()
to be "borrowed" from either the environment, or a static global variable test_argv0_dirname
copied from the executable's name.
test_argv0_dirname
is cleaned up in test_cleanup()
, which is called at the end of g_test_run()
. So these variables are OK to use in the time interval indicated by }
here:
/--- main()
|
| /--- g_test_init()
| | }
| \--- }
| }
| /--- g_test_run() }
| | /--- test-case 1 }
| | | }
| | \--- }
| | /--- test-case 2 }
| | | }
| | \--- }
| | }
| | test_cleanup()
| \---
|
| libsoup does cleanup here
\---
but afterwards, and in particular during libsoup cleanup, they might be dangling pointers.
If these functions aren't intended to be safe to call during global cleanup, then I think it would be helpful to document them as such.
Alternatively, if they're intended to be safe to call in that context, we should probably set up test_argv0_dirname
to be a deliberate leak or free it in an atexit()
hook or something, instead of freeing it in g_test_run()
.
g_test_get_filename()
is not OK to call during cleanup in any case, because it relies on the test_filename_free_list
, and it is already documented as such.