Skip to content

Commit

Permalink
dlt-user: fix memory issues
Browse files Browse the repository at this point in the history 
* improve free after failures in init
* check if freeing in some methods
* add missing free in unit tests

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
  • Loading branch information
@alexmohr
alexmohr committed Oct 16, 2024
1 parent 4e7ecb4 commit 9200809
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 39 deletions.
95 changes: 58 additions & 37 deletions src/lib/dlt_user.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,14 @@
enum InitState {
INIT_UNITIALIZED,
INIT_IN_PROGRESS,
INIT_ERROR,
INIT_DONE
};

static DltUser dlt_user;
static _Atomic enum InitState dlt_user_init_state = INIT_UNITIALIZED;
#define DLT_USER_INITALIZED (dlt_user_init_state == INIT_DONE)
#define DLT_USER_INITALIZED_NOT_FREEING (DLT_USER_INITALIZED && (dlt_user_freeing == 0))

static _Atomic int dlt_user_freeing = 0;
static bool dlt_user_file_reach_max = false;
Expand Down Expand Up @@ -465,7 +467,9 @@ DltReturnValue dlt_init(void)
{
/* process is exiting. Do not allocate new resources. */
if (dlt_user_freeing != 0) {
#ifndef DLT_UNIT_TESTS
dlt_vlog(LOG_INFO, "%s logging disabled, process is exiting\n", __func__);
#endif
/* return negative value, to stop the current log */
return DLT_RETURN_LOGGING_DISABLED;
}
Expand Down Expand Up @@ -493,7 +497,8 @@ DltReturnValue dlt_init(void)

/* Initialize common part of dlt_init()/dlt_init_file() */
if (dlt_init_common() == DLT_RETURN_ERROR) {
dlt_user_init_state = INIT_UNITIALIZED;
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -532,27 +537,37 @@ DltReturnValue dlt_init(void)
#endif
#ifdef DLT_LIB_USE_UNIX_SOCKET_IPC

if (dlt_initialize_socket_connection() != DLT_RETURN_OK)
if (dlt_initialize_socket_connection() != DLT_RETURN_OK) {
/* We could connect to the pipe, but not to the socket, which is normally */
/* open before by the DLT daemon => bad failure => return error code */
/* in case application is started before daemon, it is expected behaviour */
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

#elif defined DLT_LIB_USE_VSOCK_IPC

if (dlt_initialize_vsock_connection() != DLT_RETURN_OK)
if (dlt_initialize_vsock_connection() != DLT_RETURN_OK) {
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

#else /* DLT_LIB_USE_FIFO_IPC */

if (dlt_initialize_fifo_connection() != DLT_RETURN_OK)
if (dlt_initialize_fifo_connection() != DLT_RETURN_OK) {
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

if (dlt_receiver_init(&(dlt_user.receiver),
dlt_user.dlt_user_handle,
DLT_RECEIVE_FD,
DLT_USER_RCVBUF_MAX_SIZE) == DLT_RETURN_ERROR) {
dlt_user_init_state = INIT_UNITIALIZED;
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

Expand All @@ -567,17 +582,15 @@ DltReturnValue dlt_init(void)
#endif

if (dlt_start_threads() < 0) {
dlt_user_init_state = INIT_UNITIALIZED;
dlt_user_init_state = INIT_ERROR;
dlt_free();
return DLT_RETURN_ERROR;
}

/* prepare for fork() call */
pthread_atfork(NULL, NULL, &dlt_fork_child_fork_handler);

expectedInitState = INIT_IN_PROGRESS;
if (!(atomic_compare_exchange_strong(&dlt_user_init_state, &expectedInitState, INIT_DONE))) {
return DLT_RETURN_ERROR;
}
dlt_user_init_state = INIT_DONE;

return DLT_RETURN_OK;
}
Expand Down Expand Up @@ -741,6 +754,11 @@ DltReturnValue dlt_init_common(void)
uint32_t buffer_max_configured = 0;
uint32_t header_size = 0;

// already initialized, nothing to do
if (DLT_USER_INITALIZED) {
return DLT_RETURN_OK;
}

/* Binary semaphore for threads */
if ((pthread_attr_init(&dlt_mutex_attr) != 0) ||
(pthread_mutexattr_settype(&dlt_mutex_attr, PTHREAD_MUTEX_ERRORCHECK) != 0) ||
Expand Down Expand Up @@ -939,7 +957,7 @@ void dlt_user_atexit_handler(void)
return;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
/* close file */
dlt_log_free();
return;
Expand Down Expand Up @@ -1047,6 +1065,8 @@ DltReturnValue dlt_free(void)
return DLT_RETURN_ERROR;
}

DLT_SEM_LOCK();

dlt_stop_threads();

dlt_user_init_state = INIT_UNITIALIZED;
Expand Down Expand Up @@ -1139,13 +1159,7 @@ DltReturnValue dlt_free(void)
dlt_user.dlt_log_handle = -1;
}

/* Ignore return value */
DLT_SEM_LOCK();
dlt_receiver_free(&(dlt_user.receiver));
DLT_SEM_FREE();

/* Ignore return value */
DLT_SEM_LOCK();

dlt_user_free_buffer(&(dlt_user.resend_buffer));
dlt_buffer_free_dynamic(&(dlt_user.startup_buffer));
Expand Down Expand Up @@ -1188,7 +1202,6 @@ DltReturnValue dlt_free(void)
}

dlt_env_free_ll_set(&dlt_user.initial_ll_set);
DLT_SEM_FREE();

#ifdef DLT_NETWORK_TRACE_ENABLE
char queue_name[NAME_MAX];
Expand Down Expand Up @@ -1222,6 +1235,7 @@ DltReturnValue dlt_free(void)
trace_load_settings_count = 0;
#endif

DLT_SEM_FREE();
pthread_mutex_destroy(&dlt_mutex);

/* allow the user app to do dlt_init() again. */
Expand Down Expand Up @@ -1604,7 +1618,7 @@ DltReturnValue dlt_unregister_app_util(bool force_sending_messages)
}

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -1646,7 +1660,7 @@ DltReturnValue dlt_unregister_app_flush_buffered_logs(void)
return DLT_RETURN_ERROR;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -1991,7 +2005,7 @@ static DltReturnValue dlt_user_log_write_raw_internal(DltContextData *log, const
}

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -2050,8 +2064,10 @@ static DltReturnValue dlt_user_log_write_raw_internal(DltContextData *log, const
}
}

memcpy(log->buffer + log->size, data, length);
log->size += length;
if (data != NULL) {
memcpy(log->buffer + log->size, data, length);
log->size += length;
}

log->args_num++;

Expand Down Expand Up @@ -2084,8 +2100,8 @@ static DltReturnValue dlt_user_log_write_generic_attr(DltContextData *log, const
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -2164,7 +2180,7 @@ static DltReturnValue dlt_user_log_write_generic_formatted(DltContextData *log,
}

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -2239,8 +2255,8 @@ DltReturnValue dlt_user_log_write_uint(DltContextData *log, unsigned int data)
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -2304,7 +2320,7 @@ DltReturnValue dlt_user_log_write_uint_attr(DltContextData *log, unsigned int da
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_initialised false\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2397,7 +2413,7 @@ DltReturnValue dlt_user_log_write_ptr(DltContextData *log, void *data)
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s user_initialised false\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2425,8 +2441,8 @@ DltReturnValue dlt_user_log_write_int(DltContextData *log, int data)
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -2490,7 +2506,7 @@ DltReturnValue dlt_user_log_write_int_attr(DltContextData *log, int data, const
if (log == NULL)
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_initialised false\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2660,8 +2676,8 @@ static DltReturnValue dlt_user_log_write_sized_string_utils_attr(DltContextData
if ((log == NULL) || (text == NULL))
return DLT_RETURN_WRONG_PARAMETER;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -3955,8 +3971,8 @@ DltReturnValue dlt_user_log_send_log(DltContextData *log, const int mtype, int *

DltReturnValue ret = DLT_RETURN_OK;

if (!DLT_USER_INITALIZED) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state=%i (expected INIT_DONE), dlt_user_freeing=%i\n", __FUNCTION__, dlt_user_init_state, dlt_user_freeing);
return DLT_RETURN_ERROR;
}

Expand Down Expand Up @@ -5061,6 +5077,11 @@ void dlt_user_log_reattach_to_daemon(void)
DltContext handle;
DltContextData log_new;

if (!DLT_USER_INITALIZED_NOT_FREEING) {
return;
}


if (dlt_user.dlt_log_handle < 0) {
dlt_user.dlt_log_handle = DLT_FD_INIT;

Expand Down
17 changes: 15 additions & 2 deletions tests/gtest_dlt_user.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2031,6 +2031,8 @@ TEST(t_dlt_user_log_write_string, normal_message_truncated_because_exceed_buffer
EXPECT_EQ(DLT_RETURN_OK, dlt_free());
unsetenv(DLT_USER_ENV_LOG_MSG_BUF_LEN);
EXPECT_EQ(DLT_RETURN_OK, dlt_init());
free(expected_message);
expected_message = NULL;
}

/**
Expand Down Expand Up @@ -2430,7 +2432,7 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_1byte_in
free(message);
message = NULL;
free(expected_message);
message = NULL;
expected_message = NULL;

EXPECT_EQ(DLT_RETURN_OK, dlt_user_log_write_finish(&contextData));
EXPECT_EQ(DLT_RETURN_OK, dlt_unregister_context(&context));
Expand Down Expand Up @@ -2506,7 +2508,7 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_1byte_in
free(message);
message = NULL;
free(expected_message);
message = NULL;
expected_message = NULL;

EXPECT_EQ(DLT_RETURN_OK, dlt_user_log_write_finish(&contextData));
EXPECT_EQ(DLT_RETURN_OK, dlt_unregister_context(&context));
Expand Down Expand Up @@ -2580,6 +2582,8 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_1bytes_a
EXPECT_EQ(DLT_RETURN_OK, dlt_free());
unsetenv(DLT_USER_ENV_LOG_MSG_BUF_LEN);
EXPECT_EQ(DLT_RETURN_OK, dlt_init());
free(expected_message);
expected_message = NULL;
}

/**
Expand Down Expand Up @@ -2813,6 +2817,9 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_2bytes_a
EXPECT_EQ(DLT_RETURN_OK, dlt_free());
unsetenv(DLT_USER_ENV_LOG_MSG_BUF_LEN);
EXPECT_EQ(DLT_RETURN_OK, dlt_init());

free(expected_message);
expected_message = NULL;
}

/**
Expand Down Expand Up @@ -3046,6 +3053,9 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_3bytes_a
EXPECT_EQ(DLT_RETURN_OK, dlt_free());
unsetenv(DLT_USER_ENV_LOG_MSG_BUF_LEN);
EXPECT_EQ(DLT_RETURN_OK, dlt_init());

free(expected_message);
expected_message = NULL;
}

/**
Expand Down Expand Up @@ -3274,6 +3284,9 @@ TEST(t_dlt_user_log_write_utf8_string, normal_message_truncated_at_utf8_4bytes_a
EXPECT_EQ(DLT_RETURN_OK, dlt_free());
unsetenv(DLT_USER_ENV_LOG_MSG_BUF_LEN);
EXPECT_EQ(DLT_RETURN_OK, dlt_init());

free(expected_message);
expected_message = NULL;
}

TEST(t_dlt_user_log_write_utf8_string, nullpointer)
Expand Down

0 comments on commit 9200809

Please sign in to comment.