From 40e9724de723249aec9cccedf489c7c15b6d6879 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Sat, 7 May 2016 01:22:38 -0400 Subject: [PATCH] Switching all use of gmtime and localtime to use reentrant variants. This was causing occasional problems with the time on processes running lots of threads. The use of gmtime in the json formatter is the likely culprit due to the fact that the json formatter runs in threads. More evidence for this is that the problem only appears to exhibit when logs are being written as JSON. --- src/bro.bif | 18 +++++++++++++----- src/threading/formatters/JSON.cc | 21 ++++++++++++++------- src/util.cc | 9 ++++++++- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/bro.bif b/src/bro.bif index f21f927f92..e2baf62550 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -145,12 +145,17 @@ static void do_fmt(const char*& fmt, Val* v, ODesc* d) } time_t time = time_t(v->InternalDouble()); + struct tm t; + int is_time_fmt = *fmt == 'T'; + if ( ! localtime_r(&time, &t) ) + s.AddSP(""); + if ( ! strftime(out_buf, sizeof(out_buf), is_time_fmt ? "%Y-%m-%d-%H:%M" : "%Y-%m-%d-%H:%M:%S", - localtime(&time)) ) + &t) ) s.AddSP(""); else @@ -3140,9 +3145,11 @@ function strftime%(fmt: string, d: time%) : string %{ static char buffer[128]; - time_t t = time_t(d); + time_t timeval = time_t(d); + struct tm t; - if ( strftime(buffer, 128, fmt->CheckString(), localtime(&t)) == 0 ) + if ( ! localtime_r(&timeval, &t) || + ! strftime(buffer, 128, fmt->CheckString(), &t) ) return new StringVal(""); return new StringVal(buffer); @@ -3160,9 +3167,10 @@ function strftime%(fmt: string, d: time%) : string function strptime%(fmt: string, d: string%) : time %{ const time_t timeval = time_t(); - struct tm t = *localtime(&timeval); + struct tm t; - if ( strptime(d->CheckString(), fmt->CheckString(), &t) == NULL ) + if ( ! localtime_r(&timeval, &t) || + ! strptime(d->CheckString(), fmt->CheckString(), &t) ) { reporter->Warning("strptime conversion failed: fmt:%s d:%s", fmt->CheckString(), d->CheckString()); return new Val(0.0, TYPE_TIME); diff --git a/src/threading/formatters/JSON.cc b/src/threading/formatters/JSON.cc index 3558baee5c..45c7be3e93 100644 --- a/src/threading/formatters/JSON.cc +++ b/src/threading/formatters/JSON.cc @@ -116,21 +116,28 @@ bool JSON::Describe(ODesc* desc, Value* val, const string& name) const { char buffer[40]; char buffer2[40]; - time_t t = time_t(val->val.double_val); + time_t the_time = time_t(val->val.double_val); + struct tm t; - if ( strftime(buffer, sizeof(buffer), "%Y-%m-%dT%H:%M:%S", gmtime(&t)) > 0 ) + desc->AddRaw("\"", 1); + + if ( ! gmtime_r(&the_time, &t) || + ! strftime(buffer, sizeof(buffer), "%Y-%m-%dT%H:%M:%S", &t) ) + { + GetThread()->Error(GetThread()->Fmt("json formatter: failure getting time: (%" PRIu64 ")", val->val.double_val)); + // This was a failure, doesn't really matter what gets put here + // but it should probably stand out... + desc->Add("2000-01-01T00:00:00.000000"); + } + else { double integ; double frac = modf(val->val.double_val, &integ); snprintf(buffer2, sizeof(buffer2), "%s.%06.0fZ", buffer, frac * 1000000); - desc->AddRaw("\"", 1); desc->Add(buffer2); - desc->AddRaw("\"", 1); } - else - GetThread()->Error(GetThread()->Fmt("strftime error for JSON: %" PRIu64)); - + desc->AddRaw("\"", 1); } else if ( timestamps == TS_EPOCH ) diff --git a/src/util.cc b/src/util.cc index 0ea89beb90..1f10d7446d 100644 --- a/src/util.cc +++ b/src/util.cc @@ -571,7 +571,14 @@ const char* fmt_access_time(double t) { static char buf[256]; time_t time = (time_t) t; - strftime(buf, sizeof(buf), "%d/%m-%H:%M", localtime(&time)); + struct tm ts; + + if ( ! localtime_r(&time, &ts) ) + { + reporter->InternalError("unable to get time"); + } + + strftime(buf, sizeof(buf), "%d/%m-%H:%M", &ts); return buf; }