commit b6c3be90ff1308480635a97ab68f5aba5ee9d530
parent 1aded4de6c6a195a613d39a1f6b23f5a6ee70c1b
Author: Christophe Coustet <christophe.coustet@meso-star.com>
Date: Fri, 16 Oct 2020 15:13:17 +0200
BugFix: race condition in logging
Diffstat:
| M | src/green-compute.c | | | 193 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------- |
1 file changed, 159 insertions(+), 34 deletions(-)
diff --git a/src/green-compute.c b/src/green-compute.c
@@ -22,12 +22,77 @@
#include <rsys/cstr.h>
#include <rsys/logger.h>
#include <rsys/text_reader.h>
+#include <rsys/dynamic_array.h>
#include <math.h>
#include <string.h>
#include <ctype.h>
#include <omp.h>
+enum log_record_type {
+ RECORD_INVALID = BIT(0),
+ RECORD_UNKNOWN = BIT(1),
+ RECORD_UNUSED = BIT(2),
+ RECORD_DEFAULT = BIT(3),
+ RECORD_TYPES_UNDEF__ = BIT(4)
+};
+
+struct log_record {
+ enum log_record_type type;
+ struct str name;
+ const char* keep_line;
+ double default_value;
+};
+
+static FINLINE void
+log_record_init
+ (struct mem_allocator* allocator,
+ struct log_record* data)
+{
+ data->type = RECORD_TYPES_UNDEF__;
+ data->keep_line = NULL;
+ data->default_value = INF;
+ str_init(allocator, &data->name);
+}
+
+static FINLINE res_T
+log_record_copy
+ (struct log_record* dst,
+ struct log_record const* src)
+{
+ dst->type = src->type;
+ dst->keep_line = src->keep_line;
+ dst->default_value = src->default_value;
+ return str_copy(&dst->name, &src->name);
+}
+
+
+static FINLINE res_T
+log_record_copy_and_release
+ (struct log_record* dst,
+ struct log_record* src)
+{
+ dst->type = src->type;
+ dst->keep_line = src->keep_line;
+ dst->default_value = src->default_value;
+ return str_copy_and_release(&dst->name, &src->name);
+}
+
+static FINLINE void
+log_record_release
+ (struct log_record* data)
+{
+ str_release(&data->name);
+}
+
+#define DARRAY_NAME logs
+#define DARRAY_DATA struct log_record
+#define DARRAY_FUNCTOR_INIT log_record_init
+#define DARRAY_FUNCTOR_COPY log_record_copy
+#define DARRAY_FUNCTOR_COPY_AND_RELEASE log_record_copy_and_release
+#define DARRAY_FUNCTOR_RELEASE log_record_release
+#include <rsys/dynamic_array.h>
+
void
check_green_table_variables_use
(struct green* green)
@@ -270,33 +335,87 @@ green_compute_1
*STD = (V > 0) ? sqrt(V / (double)green->counts.ok_count) : 0;
}
+void do_log
+ (const char* file_name,
+ struct green* green,
+ struct darray_logs* logs)
+{
+ size_t i;
+ enum log_type log_type = LOG_WARNING;
+ ASSERT(file_name && green && logs);
+
+ if(darray_logs_size_get(logs) == 0) return;
+
+ FOR_EACH(i, 0, darray_logs_size_get(logs)) {
+ struct log_record* record = darray_logs_data_get(logs) + i;
+ if(record->type & (RECORD_INVALID | RECORD_UNKNOWN)) {
+ log_type = LOG_ERROR;
+ break;
+ }
+ }
+ logger_print(green->logger, log_type, "In file '%s':\n", file_name);
+ FOR_EACH(i, 0, darray_logs_size_get(logs)) {
+ struct log_record* record = darray_logs_data_get(logs) + i;
+ switch (record->type) {
+ case RECORD_INVALID:
+ logger_print(green->logger, LOG_ERROR,
+ "In line '%s':\n\tInvalid data (missing name)\n",
+ record->keep_line);
+ break;
+ case RECORD_UNKNOWN:
+ logger_print(green->logger, LOG_ERROR,
+ "In line '%s':\n\tUnknown variable name: '%s'\n",
+ record->keep_line, str_cget(&record->name));
+ break;
+ case RECORD_UNUSED:
+ logger_print(green->logger, LOG_WARNING,
+ "In line '%s':\n\tAttempt to change unused variable '%s'.\n",
+ record->keep_line, str_cget(&record->name));
+ break;
+ case RECORD_DEFAULT:
+ logger_print(green->logger, LOG_WARNING,
+ "In line '%s':\n\tChange variable '%s' to its default value %g.\n",
+ record->keep_line, str_cget(&record->name), record->default_value);
+ break;
+ default:
+ FATAL("error:" STR(__FILE__) ":" STR(__LINE__)": Invalid type.\n");
+ }
+ }
+}
+
static res_T
parse_line
(const char* file_name,
const int64_t idx,
struct green* green,
- struct applied_settings_elt* settings)
+ struct applied_settings_elt* settings,
+ struct darray_logs* logs)
{
res_T res = RES_OK;
- int name_count = 0;
+ int name_count;
struct str name;
char* tk = NULL;
char* tok_ctx = NULL;
- const struct str* keep_line = darray_str_cdata_get(&green->settings) + idx;
+ const struct str* keep_line;
struct str line;
- int unused = 0, same = 0;
-
ASSERT(file_name && green);
+ keep_line = darray_str_cdata_get(&green->settings) + idx;
str_init(green->allocator, &line);
str_init(green->allocator, &name);
ERR(str_set(&line, str_cget(keep_line)));
/* At least one name=value, no name without value */
- for(;;) {
+ for(name_count = 0; ; name_count++) {
struct variable_data* vd;
+ struct log_record record;
double prev;
+
+ if(res != RES_OK) goto error;
+
+ record.keep_line = str_cget(keep_line); /* Just in case */
+
tk = strtok_r(name_count ? NULL : str_get(&line), "=", &tok_ctx);
if(tk) {
char* c;
@@ -311,52 +430,54 @@ parse_line
if(!tk) {
if(name_count == 0) {
/* At least 1 name */
- logger_print(green->logger, LOG_ERROR,
- "Invalid data (missing name)\n");
+ record.type = RECORD_INVALID;
+ str_init(green->allocator, &record.name);
res = RES_BAD_ARG;
- goto error;
+ goto push_record;
}
else break;
}
ERR(str_set(&name, tk));
CHK_TOK(strtok_r(NULL, " \t", &tok_ctx), "value");
vd = htable_variable_ptr_find(&green->variable_ptrs, &name);
- if(!vd || !vd->used) {
- /* Name should exist */
- if(!vd) {
- logger_print(green->logger, LOG_ERROR,
- "Invalid variable name: '%s'\n",
- str_cget(&name));
- res = RES_BAD_ARG;
- goto error;
- }
- logger_print(green->logger, LOG_WARNING,
- "Attempt to change unused variable '%s'.\n", str_cget(&name));
- unused = 1;
+ if(!vd) {
+ record.type = RECORD_UNKNOWN;
+ str_init(green->allocator, &record.name);
+ ERR(str_copy(&record.name, &name));
+ res = RES_BAD_ARG;
+ goto push_record;
}
+ if(!vd->used) {
+ record.type = RECORD_UNUSED;
+ str_init(green->allocator, &record.name);
+ ERR(str_copy(&record.name, &name));
+ goto push_record;
+ }
+ /* Variable exists and is used in Green function: check if really changed */
prev = ((double*)settings)[vd->idx];
ERR(cstr_to_double(tk, ((double*)settings) + vd->idx));
if(prev == ((double*)settings)[vd->idx]) {
- logger_print(green->logger, LOG_WARNING,
- "Change variable '%s' to its default value %g.\n",
- str_cget(&name), prev);
- same = 1;
+ record.type = RECORD_DEFAULT;
+ str_init(green->allocator, &record.name);
+ ERR(str_copy(&record.name, &name));
+ record.default_value = prev;
+ goto push_record;
+ }
+ continue;
+push_record:
+ #pragma omp critical
+ {
+ res_T tmp_res = darray_logs_push_back(logs, &record);
+ str_release(&record.name);
+ if(res == RES_OK) res = tmp_res;
}
- name_count++;
}
- if(unused || same)
- logger_print(green->logger, LOG_WARNING,
- "In file '%s':\n\t%s\n",
- file_name, str_cget(keep_line));
end:
str_release(&line);
str_release(&name);
return res;
error:
- logger_print(green->logger, LOG_ERROR,
- "Invalid line in file '%s':\n", file_name);
- logger_print(green->logger, LOG_ERROR, "%s\n", str_cget(keep_line));
goto end;
}
@@ -369,6 +490,7 @@ green_compute
size_t sz;
int64_t i;
struct applied_settings_elt** settings = NULL;
+ struct darray_logs logs;
ASSERT(green && in_name);
sz = darray_str_size_get(&green->settings);
@@ -379,6 +501,7 @@ green_compute
res = RES_MEM_ERR;
goto error;
}
+ darray_logs_init(green->allocator, &logs);
omp_set_num_threads((int)green->nthreads);
#pragma omp parallel for schedule(static)
@@ -401,7 +524,7 @@ green_compute
/* Apply settings */
applied_settings_set_defaults(green, settings[t]);
- tmp_res = parse_line(in_name, i, green, settings[t]);
+ tmp_res = parse_line(in_name, i, green, settings[t], &logs);
if(tmp_res != RES_OK) {
res = tmp_res;
continue;
@@ -412,10 +535,12 @@ green_compute
if(res != RES_OK) goto error;
exit:
+ do_log(in_name, green, &logs);
if(settings) {
FOR_EACH(i, 0, green->nthreads) MEM_RM(green->allocator, settings[i]);
MEM_RM(green->allocator, settings);
}
+ darray_logs_release(&logs);
return res;
error:
goto exit;
