Skip to content

Commit

Permalink
dlt-system: move journal reading to its own thread (#471)
Browse files Browse the repository at this point in the history
Reading from journal uses an endless loop which only exits
when no new messages are read from the journal.
This is a problem when the local buffer of the application
gets filled i.e. due dlt-daemon being busy and not accepting
current messages.
In this case the thread waits for 500ms and tries again to
read messages from the journal.
When new messages are available they also will be written to dlt.
If this cycle continues for a while it is possible that other tasks,
which use the same thread and are controlled via a timer, are not
executed anymore as the funtion `get_journal_msg` will not exit.
This will lead to termination through the systemd watchdog,
as sd_notify is not called anymore.
To circumvent this issue, reading logs from journald is moved
into a separate thread.

Furthermore this commit fixes some invalid free when no configuration
is passed to dlt-system as well as adding missing initialization which
also can lead to crashes.
Although these issues are not responsible for the crash on the system,
as they will only happen on application shutdown.

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
  • Loading branch information
alexmohr authored Aug 2, 2023
1 parent a4ece76 commit 9352f0d
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 21 deletions.
24 changes: 22 additions & 2 deletions src/system/dlt-system-journal.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <systemd/sd-journal.h>
#include <systemd/sd-id128.h>
#include <inttypes.h> /* for PRI formatting macro */
#include <assert.h>


#define DLT_SYSTEM_JOURNAL_BUFFER_SIZE 256
Expand Down Expand Up @@ -389,9 +390,28 @@ void register_journal_fd(sd_journal **j, struct pollfd *pollfd, int i, DltSyste
*j = j_tmp;
}

void journal_fd_handler(sd_journal *j, DltSystemConfiguration *config)
void* journal_thread(void* journalParams)
{
get_journal_msg(j, config);
struct journal_fd_params* params = (struct journal_fd_params*)journalParams;

int ready;
while (*params->quit == 0) {
ready = poll(params->journalPollFd, 1, -1);
if (ready == -1) {
DLT_LOG(dltsystem, DLT_LOG_ERROR, DLT_STRING("Error while poll. Exit with: "),
DLT_STRING(strerror(ready)));
continue;
}

if(params->journalPollFd->revents & POLLIN) {
if (sd_journal_process(params->j) == SD_JOURNAL_APPEND) {
get_journal_msg(params->j, params->config);
}
}
}

// void* is only necessary to fulfill pthread_create signature
return NULL;
}

#endif /* DLT_SYSTEMD_JOURNAL_ENABLE */
4 changes: 3 additions & 1 deletion src/system/dlt-system-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ void usage(char *prog_name)
void init_cli_options(DltSystemCliOptions *options)
{
options->ConfigurationFileName = strdup(DEFAULT_CONF_FILE);
options->freeConfigFileName = 0;
options->Daemonize = 0;
}

Expand All @@ -103,6 +104,7 @@ int read_command_line(DltSystemCliOptions *options, int argc, char *argv[])
case 'c':
{
options->ConfigurationFileName = malloc(strlen(optarg) + 1);
options->freeConfigFileName = 1;
MALLOC_ASSERT(options->ConfigurationFileName);
strcpy(options->ConfigurationFileName, optarg); /* strcpy unritical here, because size matches exactly the size to be copied */
break;
Expand Down Expand Up @@ -419,7 +421,7 @@ int read_configuration_file(DltSystemConfiguration *config, char *file_name)
void cleanup_config(DltSystemConfiguration *config, DltSystemCliOptions *options)
{
/* command line options */
if ((options->ConfigurationFileName) != NULL)
if ((options->ConfigurationFileName) != NULL && options->freeConfigFileName)
{
free(options->ConfigurationFileName);
options->ConfigurationFileName = NULL;
Expand Down
44 changes: 30 additions & 14 deletions src/system/dlt-system-process-handling.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include <poll.h>
#include <sys/timerfd.h>
#include <time.h>
#include <errno.h>

DLT_IMPORT_CONTEXT(dltsystem)
DLT_IMPORT_CONTEXT(syslogContext)
Expand Down Expand Up @@ -121,21 +122,28 @@ int daemonize()
}

/* Unregisters all DLT Contexts and closes all file descriptors */
void cleanup_processes(struct pollfd *pollfd, sd_journal *j, DltSystemConfiguration *config)
void cleanup_processes(struct pollfd *pollfd, struct pollfd *journalPollFd, sd_journal *j, DltSystemConfiguration *config)
{
//Syslog cleanup
if (config->Syslog.Enable)
DLT_UNREGISTER_CONTEXT(syslogContext);

//Journal cleanup
#if defined(DLT_SYSTEMD_JOURNAL_ENABLE)
if (config->Journal.Enable)
DLT_UNREGISTER_CONTEXT(journalContext);
if(j != NULL)
sd_journal_close(j);

if(journalPollFd->fd > 0)
close(journalPollFd->fd);
#else
// silence warnings
(void)j;
(void)journalPollFd->fd;
#endif

//Logfile cleanup
//Logfile cleanup
if (config->LogFile.Enable) {
for (int i = 0; i < config->LogFile.Count; i++)
DLT_UNREGISTER_CONTEXT(logfileContext[i]);
Expand Down Expand Up @@ -245,11 +253,21 @@ void start_dlt_system_processes(DltSystemConfiguration *config)

//init FD for Journal
sd_journal *j = NULL;
struct pollfd journalPollFd;
#if defined(DLT_SYSTEMD_JOURNAL_ENABLE)
pthread_t journalThreadHandle;
if (config->Journal.Enable) {
register_journal_fd(&j, pollfd, fdcnt, config);
fdType[fdcnt] = fdType_journal;
fdcnt++;
register_journal_fd(&j, &journalPollFd, 0, config);
struct journal_fd_params params = {
&quit,
&journalPollFd,
j,
config
};
if (pthread_create(&journalThreadHandle, NULL, &journal_thread, (void*)&params) != 0) {
DLT_LOG(dltsystem, DLT_LOG_ERROR, DLT_STRING("Failed to create journal_thread thread "),
DLT_STRING(strerror(errno)));
}
}
#endif

Expand Down Expand Up @@ -286,13 +304,6 @@ void start_dlt_system_processes(DltSystemConfiguration *config)
else if (fdType[i] == fdType_timer) {
timer_fd_handler(pollfd[i].fd, config);
}
#if defined(DLT_SYSTEMD_JOURNAL_ENABLE)
else if((fdType[i] == fdType_journal) && (j != NULL)) {
if(sd_journal_process(j) == SD_JOURNAL_APPEND) {
journal_fd_handler(j, config);
}
}
#endif
#if defined(DLT_FILETRANSFER_ENABLE)
else if (fdType[i] == fdType_filetransfer) {
filetransfer_fd_handler(config);
Expand All @@ -311,7 +322,12 @@ void start_dlt_system_processes(DltSystemConfiguration *config)
}
}

cleanup_processes(pollfd, j, config);
#if defined(DLT_SYSTEMD_JOURNAL_ENABLE)
if (config->Journal.Enable) {
pthread_join(journalThreadHandle, NULL);
}
#endif
cleanup_processes(pollfd, &journalPollFd, j, config);
}

void dlt_system_signal_handler(int sig)
Expand Down
14 changes: 10 additions & 4 deletions src/system/dlt-system.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@
#define MAX_LINE 1024

/** Total number of file descriptors needed for processing all features:
* - Journal file descriptor
* - Syslog file descriptor
* - Timer file descriptor for processing LogFile and LogProcesses every second
* - Inotify file descriptor for FileTransfer
* - Timer file descriptor for Watchdog
*/
#define MAX_FD_NUMBER 5
#define MAX_FD_NUMBER 4

/* Macros */
#define MALLOC_ASSERT(x) if (x == NULL) { \
Expand All @@ -89,8 +88,7 @@

/* enum for classification of FD */
enum fdType {
fdType_journal = 0,
fdType_syslog,
fdType_syslog = 0,
fdType_filetransfer,
fdType_timer,
fdType_watchdog,
Expand All @@ -104,6 +102,7 @@ enum fdType {
/* Command line options */
typedef struct {
char *ConfigurationFileName;
int freeConfigFileName;
int Daemonize;
} DltSystemCliOptions;

Expand Down Expand Up @@ -218,6 +217,13 @@ void watchdog_fd_handler(int fd, int* received_message_since_last_watchdog_inter
void watchdog_fd_handler(int fd);
#endif
void journal_fd_handler(sd_journal *j, DltSystemConfiguration *config);
struct journal_fd_params {
volatile uint8_t* quit;
struct pollfd* journalPollFd;
sd_journal *j;
DltSystemConfiguration *config;
};
void *journal_thread(void* journalParams);
void syslog_fd_handler(int syslogSock);

#endif /* DLT_SYSTEM_H_ */

0 comments on commit 9352f0d

Please sign in to comment.