lsl_pull_sample_str memory leak scenarios

[jseyelink]

Description

The documentation in include/lsl/inlet.h about the function lsl_pull_sample_str has this to say,

param[out] buffer A pointer to hold the resulting values.
note If the timeout expires before a new sample was received the function returns 0.0
return The capture time of the sample on the remote machine, or 0.0 if no new sample was available.

What is left unsaid is that lsl_pull_sample_str will allocate a string buffer in heap memory for each element of buffer, so that buffer[ i ] points to the i-th string buffer. Therefore, it is the responsibility of the calling function to free all string buffers once they have been processed.

A user of liblsl may reasonably do as follows with a one-channel Marker inlet stream,

double  time ;
int32_t  error ;
char * marker ;

while  (  <continue iterating>  )  /* Main event loop */
{
    time = lsl_pull_sample_str (  inlet ,  &marker ,  1 ,  0.0 ,  &error ) ;
    
    if  (  0.0 != time  )  /* A marker was returned. */
    {
        <process marker string>
        free (  marker  ) ;
    }
}

Indeed, this is the pattern that the following lsl-leak-test program will use,

CMakeLists.txt
main.c

A simple way to monitor the memory consumption of that program is to open another terminal in which the top command is running. The <Shift+m> keyboard shortcut will sort the processes by their memory allocation. <Shift+p> will order the processes by CPU use.

There are several tests that demonstrate an unexpected memory leak scenario.

Test 1

Execute $ lsl-leak-test 0, where the command line argument is zero. This tells the program to free the marker pointer only when lsl_pull_sample_str has returned a non-zero value. That function will be called at a high rate. The consequence is that the process will accumulate a significant proportion of the system's available memory in a matter of seconds. Be careful to terminate this process before the system freezes.

Test 2

Execute $ lsl-leak-test 1, where the command line argument is a non-zero integer. Now the program will free the marker pointer after every call to lsl_pull_sample_str, regardless of what it returned. The program can run indefinitely without accumulating memory.

Test 3

In the previous tests, lsl-leak-test was linked to the system installation of liblsl. Now build a local test version of liblsl with the following changes to the implementation of lsl_pull_sample_str from src/lsl_inlet_c.cpp.

LIBLSL_C_API double lsl_pull_sample_str(
    lsl_inlet in, char **buffer, int32_t buffer_elements, double timeout, int32_t *ec) {
    if (ec) *ec = lsl_no_error;
    try {
	    // capture output in a temporary string buffer
	    std::vector<std::string> tmp;
	    double result = in->pull_sample(tmp, timeout);
	    if (buffer_elements < (int)tmp.size())
		    throw std::range_error(
			    "The provided buffer has fewer elements than the stream's number of channels.");
	    // allocate memory and copy over into buffer
	    if  ( 0.0 < result )  /***TESTING***TESTING***TESTING***TESTING***TESTING***/
		    for (std::size_t k = 0; k < tmp.size(); k++) {
			    buffer[k] = (char *)malloc(tmp[k].size() + 1);
			    if (buffer[k] == nullptr) {
				    for (std::size_t k2 = 0; k2 < k; k2++) free(buffer[k2]);
				    if (ec) *ec = lsl_internal_error;
				    return 0.0;
			    }
			    memcpy(buffer[k], tmp[k].data(), tmp[k].size());
			    buffer[k][tmp[k].size()] = '\0';
		    }
	    else  for (std::size_t k = 0; k < tmp.size(); k++) buffer[k] = nullptr;  /***TESTING***/
	    return result;
    } LSL_STORE_EXCEPTION_IN(ec)
    return 0.0;
}

Notice that malloc will only be called when result is greater than zero. LD_LIBRARY_PATH can be used to link lsl-leak-test to the local test build of liblsl. When this is done, lsl-leak-test can be called with a command line argument of zero; i.e. the program only frees the marker pointer when lsl_pull_sample_str returns a non-zero value.

However, unlike in Test 1, there is no accumulation of memory by the lsl-leak-test process.

Discussion

Naïve use of the lsl_pull_sample_str function can cause an aggressive memory leak, in at least two ways.

1. The calling function never frees the marker string pointer, under any circumstance.
2. The calling function only frees the marker string pointer when lsl_pull_sample_str returns a non-zero value.

The first problem may be addressed by adding very clear instruction to the documentation that it is the calling function's responsibility to free the marker string. Resolution of the second can be achieved by a similar fix to that as shown above.

Versions

Output of lslver command.

Official release

LSL version: 117
git:64988c6a14b8dc3b3f270ece58eab4f480bfab43/branch:refs/tags/v1.17.7/build:Release/compiler:GNU-11.4.0/link:SHARED
8542.044398

Local test build

LSL version: 117
git:v1.17.7/branch:dev/build:/compiler:GNU-11.4.0/link:SHARED
8648.887453

Test and build system

Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.5 LTS
Release:	22.04
Codename:	jammy
Linux display-desktop 6.8.0-107-generic #107~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Mar 18 23:40:43 UTC  x86_64 x86_64 x86_64 GNU/Linux

cmake version 4.3.2

gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

[jseyelink]

Strictly speaking, any string that lsl_pull_sample_str returns should always be released using lsl_destroy_string, rather than a direct call to free. It can happen that an application is linked to a different version of the C standard library than a specific build of liblsl. In this case, lsl_pull_sample_str may call malloc from one version of the C library, while the application calls free from a different one. The result can be delightfully undefined!

This is especially the case in Windows. If lsl.dll and the application are built using the Debug configuration in different versions of Visual Studio, then a Debug assertion may be triggered by _CrtIsValidHeapPointer; the assertion may generate an error dialogue, or it may simply freeze the application without giving any reason. Although, running the application from Visual Studio should at least halt the program and highlight the call to free that triggered the assertion.

It would be helpful if the documentation for lsl_pull_sample_str were to mention that the calling program has the responsibility of releasing all marker strings with calls to lsl_destroy_string. The documentation for the latter function might also refer to lsl_pull_sample_str.