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 <[email protected]>
  • Loading branch information
@alexmohr
alexmohr committed Jul 17, 2024
1 parent 358ab08 commit 7cdbe8d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 28 deletions.
73 changes: 47 additions & 26 deletions src/lib/dlt_user.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ enum InitState {
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 @@ -457,7 +458,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 @@ -485,7 +488,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_DONE;
dlt_free();
return DLT_RETURN_ERROR;
}

Expand All @@ -507,27 +511,37 @@ DltReturnValue dlt_init(void)

#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_DONE;
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_DONE;
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_DONE;
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_DONE;
dlt_free();
return DLT_RETURN_ERROR;
}

Expand All @@ -542,17 +556,15 @@ DltReturnValue dlt_init(void)
#endif

if (dlt_start_threads() < 0) {
dlt_user_init_state = INIT_UNITIALIZED;
dlt_user_init_state = INIT_DONE;
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 @@ -716,6 +728,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 @@ -1018,6 +1035,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 @@ -1110,13 +1129,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 @@ -1159,7 +1172,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 All @@ -1184,6 +1196,8 @@ DltReturnValue dlt_free(void)

pthread_cond_destroy(&mq_init_condition);
#endif /* DLT_NETWORK_TRACE_ENABLE */

DLT_SEM_FREE();
pthread_mutex_destroy(&dlt_mutex);

/* allow the user app to do dlt_init() again. */
Expand Down Expand Up @@ -2008,7 +2022,9 @@ static DltReturnValue dlt_user_log_write_raw_internal(DltContextData *log, const
}
}

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

log->args_num++;
Expand Down Expand Up @@ -2042,7 +2058,7 @@ static DltReturnValue dlt_user_log_write_generic_attr(DltContextData *log, 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_init_state != INIT_DONE\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2197,7 +2213,7 @@ DltReturnValue dlt_user_log_write_uint(DltContextData *log, unsigned int data)
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_init_state != INIT_DONE\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2262,7 +2278,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 @@ -2355,7 +2371,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 @@ -2383,7 +2399,7 @@ DltReturnValue dlt_user_log_write_int(DltContextData *log, int data)
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_init_state != INIT_DONE\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -2448,7 +2464,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 @@ -2618,7 +2634,7 @@ 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) {
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -3902,7 +3918,7 @@ DltReturnValue dlt_user_log_send_log(DltContextData *log, int mtype)

DltReturnValue ret = DLT_RETURN_OK;

if (!DLT_USER_INITALIZED) {
if (!DLT_USER_INITALIZED_NOT_FREEING) {
dlt_vlog(LOG_WARNING, "%s dlt_user_init_state != INIT_DONE\n", __FUNCTION__);
return DLT_RETURN_ERROR;
}
Expand Down Expand Up @@ -4887,6 +4903,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 7cdbe8d

Please sign in to comment.