From ab63fca3d5909dd1918272749b84a3b45c286d24 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Thu, 27 Nov 2014 08:04:07 +0100 Subject: [PATCH] Make the config parser thread-safe fixes #7822 --- lib/base/threadpool.cpp | 2 + lib/cli/daemoncommand.cpp | 4 ++ lib/config/config_lexer.ll | 102 +++++++--------------------------- lib/config/config_parser.yy | 31 +++-------- lib/config/configcompiler.cpp | 34 +++++++++--- lib/config/configcompiler.hpp | 7 +++ lib/config/expression.hpp | 23 ++++++++ 7 files changed, 90 insertions(+), 113 deletions(-) diff --git a/lib/base/threadpool.cpp b/lib/base/threadpool.cpp index 2b2682860..0f6d44e46 100644 --- a/lib/base/threadpool.cpp +++ b/lib/base/threadpool.cpp @@ -69,6 +69,8 @@ void ThreadPool::Stop(void) } m_ThreadGroup.join_all(); + + m_Stopped = false; } /** diff --git a/lib/cli/daemoncommand.cpp b/lib/cli/daemoncommand.cpp index c269214e9..85dbfe31b 100644 --- a/lib/cli/daemoncommand.cpp +++ b/lib/cli/daemoncommand.cpp @@ -201,6 +201,8 @@ static void SigHupHandler(int) static bool Daemonize(void) { #ifndef _WIN32 + Application::GetTP().Stop(); + pid_t pid = fork(); if (pid == -1) { return false; @@ -231,6 +233,8 @@ static bool Daemonize(void) Application::Exit(0); } + + Application::GetTP().Start(); #endif /* _WIN32 */ return true; diff --git a/lib/config/config_lexer.ll b/lib/config/config_lexer.ll index 8f17c33b4..90b523654 100644 --- a/lib/config/config_lexer.ll +++ b/lib/config/config_lexer.ll @@ -60,54 +60,6 @@ do { \ do { \ result = yyextra->ReadInput(buf, max_size); \ } while (0) - -extern int ignore_newlines; - -struct lex_buf { - char *buf; - size_t size; -}; - -static void lb_init(lex_buf *lb) -{ - lb->buf = NULL; - lb->size = 0; -} - -/*static void lb_cleanup(lex_buf *lb) -{ - free(lb->buf); -}*/ - -static void lb_append_char(lex_buf *lb, char new_char) -{ - const size_t block_size = 64; - - size_t old_blocks = (lb->size + (block_size - 1)) / block_size; - size_t new_blocks = ((lb->size + 1) + (block_size - 1)) / block_size; - - if (old_blocks != new_blocks) { - char *new_buf = (char *)realloc(lb->buf, new_blocks * block_size); - - if (new_buf == NULL && new_blocks > 0) - throw std::bad_alloc(); - - lb->buf = new_buf; - } - - lb->size++; - lb->buf[lb->size - 1] = new_char; -} - -static char *lb_steal(lex_buf *lb) -{ - lb_append_char(lb, '\0'); - - char *buf = lb->buf; - lb->buf = NULL; - lb->size = 0; - return buf; -} %} %option reentrant noyywrap yylineno @@ -120,25 +72,19 @@ static char *lb_steal(lex_buf *lb) %x HEREDOC %% - lex_buf string_buf; - -\" { lb_init(&string_buf); BEGIN(STRING); } +\" { yyextra->m_LexBuffer.str(""); yyextra->m_LexBuffer.clear(); BEGIN(STRING); } \" { BEGIN(INITIAL); - lb_append_char(&string_buf, '\0'); - - yylval->text = lb_steal(&string_buf); + std::string str = yyextra->m_LexBuffer.str(); + yylval->text = strdup(str.c_str()); return T_STRING; } \n { - std::ostringstream msgbuf; - msgbuf << "Unterminated string found: " << *yylloc; - ConfigCompilerContext::GetInstance()->AddMessage(true, msgbuf.str()); - BEGIN(INITIAL); + BOOST_THROW_EXCEPTION(ConfigError("Unterminated string literal") << errinfo_debuginfo(*yylloc)); } \\[0-7]{1,3} { @@ -149,50 +95,45 @@ static char *lb_steal(lex_buf *lb) if (result > 0xff) { /* error, constant is out-of-bounds */ - std::ostringstream msgbuf; - msgbuf << "Constant is out-of-bounds: " << yytext << " " << *yylloc; - ConfigCompilerContext::GetInstance()->AddMessage(true, msgbuf.str()); + BOOST_THROW_EXCEPTION(ConfigError("Constant is out of bounds: " + String(yytext)) << errinfo_debuginfo(*yylloc)); } - lb_append_char(&string_buf, result); + yyextra->m_LexBuffer << static_cast(result); } \\[0-9]+ { /* generate error - bad escape sequence; something * like '\48' or '\0777777' */ - std::ostringstream msgbuf; - msgbuf << "Bad escape sequence found: " << yytext << " " << *yylloc; - ConfigCompilerContext::GetInstance()->AddMessage(true, msgbuf.str()); + BOOST_THROW_EXCEPTION(ConfigError("Bad escape sequence found: " + String(yytext)) << errinfo_debuginfo(*yylloc)); } -\\n { lb_append_char(&string_buf, '\n'); } -\\t { lb_append_char(&string_buf, '\t'); } -\\r { lb_append_char(&string_buf, '\r'); } -\\b { lb_append_char(&string_buf, '\b'); } -\\f { lb_append_char(&string_buf, '\f'); } -\\(.|\n) { lb_append_char(&string_buf, yytext[1]); } +\\n { yyextra->m_LexBuffer << "\n"; } +\\t { yyextra->m_LexBuffer << "\t"; } +\\r { yyextra->m_LexBuffer << "\r"; } +\\b { yyextra->m_LexBuffer << "\b"; } +\\f { yyextra->m_LexBuffer << "\f"; } +\\(.|\n) { yyextra->m_LexBuffer << yytext[1]; } [^\\\n\"]+ { char *yptr = yytext; while (*yptr) - lb_append_char(&string_buf, *yptr++); + yyextra->m_LexBuffer << *yptr++; } -\{\{\{ { lb_init(&string_buf); BEGIN(HEREDOC); } +\{\{\{ { yyextra->m_LexBuffer.str(""); yyextra->m_LexBuffer.clear(); BEGIN(HEREDOC); } \}\}\} { BEGIN(INITIAL); - lb_append_char(&string_buf, '\0'); - - yylval->text = lb_steal(&string_buf); + std::string str = yyextra->m_LexBuffer.str(); + yylval->text = strdup(str.c_str()); return T_STRING; } -(.|\n) { lb_append_char(&string_buf, yytext[0]); } +(.|\n) { yyextra->m_LexBuffer << yytext[0]; } { "/*" BEGIN(C_COMMENT); @@ -205,10 +146,7 @@ static char *lb_steal(lex_buf *lb) } <> { - std::ostringstream msgbuf; - msgbuf << "End-of-file while in comment: " << yytext << " " << *yylloc; - ConfigCompilerContext::GetInstance()->AddMessage(true, msgbuf.str()); - yyterminate(); + BOOST_THROW_EXCEPTION(ConfigError("End-of-file while in comment") << errinfo_debuginfo(*yylloc)); } @@ -294,7 +232,7 @@ in return T_IN; \> return T_GREATER_THAN; } -[\r\n]+ { yycolumn -= strlen(yytext) - 1; if (!ignore_newlines) return T_NEWLINE; } +[\r\n]+ { yycolumn -= strlen(yytext) - 1; if (!yyextra->m_IgnoreNewlines) return T_NEWLINE; } <> { if (!yyextra->m_Eof) { yyextra->m_Eof = true; return T_NEWLINE; } else { yyterminate(); } } . return yytext[0]; diff --git a/lib/config/config_parser.yy b/lib/config/config_parser.yy index 1d6679c66..32b26dbd2 100644 --- a/lib/config/config_parser.yy +++ b/lib/config/config_parser.yy @@ -69,8 +69,6 @@ do { \ using namespace icinga; -int ignore_newlines = 0; - template static void MakeRBinaryOp(Expression** result, Expression *left, Expression *right, DebugInfo& diLeft, DebugInfo& diRight) { @@ -230,32 +228,21 @@ int yylex(YYSTYPE *lvalp, YYLTYPE *llocp, void *scanner); void yyerror(YYLTYPE *locp, std::vector *, ConfigCompiler *, const char *err) { - std::ostringstream message; - message << *locp << ": " << err; - ConfigCompilerContext::GetInstance()->AddMessage(true, message.str(), *locp); + BOOST_THROW_EXCEPTION(ConfigError(err) << errinfo_debuginfo(*locp)); } int yyparse(std::vector *elist, ConfigCompiler *context); Expression *ConfigCompiler::Compile(void) { - try { - std::vector elist; + std::vector elist; - if (yyparse(&elist, this) != 0) - return NULL; + if (yyparse(&elist, this) != 0) + return NULL; - DictExpression *expr = new DictExpression(elist); - expr->MakeInline(); - return expr; - } catch (const ConfigError& ex) { - const DebugInfo *di = boost::get_error_info(ex); - ConfigCompilerContext::GetInstance()->AddMessage(true, ex.what(), di ? *di : DebugInfo()); - } catch (const std::exception& ex) { - ConfigCompilerContext::GetInstance()->AddMessage(true, DiagnosticInformation(ex)); - } - - return NULL; + DictExpression *expr = new DictExpression(elist); + expr->MakeInline(); + return expr; } #define scanner (context->GetScanner()) @@ -788,11 +775,11 @@ rterm_without_indexer: T_STRING } | '(' { - ignore_newlines++; + context->m_IgnoreNewlines++; } rterm ')' { - ignore_newlines--; + context->m_IgnoreNewlines--; $$ = $3; } | rterm T_LOGICAL_OR rterm { MakeRBinaryOp(&$$, $1, $3, @1, @3); } diff --git a/lib/config/configcompiler.cpp b/lib/config/configcompiler.cpp index 925e51fb9..1e4f47bd8 100644 --- a/lib/config/configcompiler.cpp +++ b/lib/config/configcompiler.cpp @@ -41,7 +41,7 @@ std::vector ConfigCompiler::m_IncludeSearchDirs; * @param zone The zone. */ ConfigCompiler::ConfigCompiler(const String& path, std::istream *input, const String& zone) - : m_Path(path), m_Input(input), m_Zone(zone), m_Eof(false) + : m_Path(path), m_Input(input), m_Zone(zone), m_Eof(false), m_IgnoreNewlines(0) { InitializeScanner(); } @@ -52,6 +52,7 @@ ConfigCompiler::ConfigCompiler(const String& path, std::istream *input, const St ConfigCompiler::~ConfigCompiler(void) { DestroyScanner(); + delete m_Input; } /** @@ -173,6 +174,17 @@ void ConfigCompiler::HandleLibrary(const String& library) (void) Utility::LoadExtensionLibrary(library); } +void ConfigCompiler::CompileHelper(void) +{ + try { + m_Promise.set_value(boost::shared_ptr(Compile())); + } catch (...) { + m_Promise.set_exception(boost::current_exception()); + } + + delete this; +} + /** * Compiles a stream. * @@ -186,8 +198,12 @@ Expression *ConfigCompiler::CompileStream(const String& path, std::istream *stre stream->exceptions(std::istream::badbit); - ConfigCompiler ctx(path, stream, zone); - return ctx.Compile(); + ConfigCompiler* ctx = new ConfigCompiler(path, stream, zone); + + boost::shared_future > ftr = boost::shared_future >(ctx->m_Promise.get_future()); + + Utility::QueueAsyncCallback(boost::bind(&ConfigCompiler::CompileHelper, ctx)); + return new FutureExpression(ftr); } /** @@ -200,10 +216,10 @@ Expression *ConfigCompiler::CompileFile(const String& path, const String& zone) { CONTEXT("Compiling configuration file '" + path + "'"); - std::ifstream stream; - stream.open(path.CStr(), std::ifstream::in); + std::ifstream *stream = new std::ifstream(); + stream->open(path.CStr(), std::ifstream::in); - if (!stream) + if (!*stream) BOOST_THROW_EXCEPTION(posix_error() << boost::errinfo_api_function("std::ifstream::open") << boost::errinfo_errno(errno) @@ -212,7 +228,7 @@ Expression *ConfigCompiler::CompileFile(const String& path, const String& zone) Log(LogInformation, "ConfigCompiler") << "Compiling config file: " << path; - return CompileStream(path, &stream, zone); + return CompileStream(path, stream, zone); } /** @@ -224,8 +240,8 @@ Expression *ConfigCompiler::CompileFile(const String& path, const String& zone) */ Expression *ConfigCompiler::CompileText(const String& path, const String& text, const String& zone) { - std::stringstream stream(text); - return CompileStream(path, &stream, zone); + std::stringstream *stream = new std::stringstream(text); + return CompileStream(path, stream, zone); } /** diff --git a/lib/config/configcompiler.hpp b/lib/config/configcompiler.hpp index 31bf77d20..d41cdbfb6 100644 --- a/lib/config/configcompiler.hpp +++ b/lib/config/configcompiler.hpp @@ -81,6 +81,8 @@ public: void *GetScanner(void) const; private: + boost::promise > m_Promise; + String m_Path; std::istream *m_Input; String m_Zone; @@ -88,6 +90,9 @@ private: void *m_Scanner; bool m_Eof; + int m_IgnoreNewlines; + std::ostringstream m_LexBuffer; + std::stack m_RuleLists; ConfigType::Ptr m_Type; @@ -105,6 +110,8 @@ private: void InitializeScanner(void); void DestroyScanner(void); + void CompileHelper(void); + friend int ::yylex(YYSTYPE *context, icinga::DebugInfo *di, yyscan_t scanner); friend int ::yyparse(std::vector *elist, ConfigCompiler *context); }; diff --git a/lib/config/expression.hpp b/lib/config/expression.hpp index e98c69745..d89a5f3e5 100644 --- a/lib/config/expression.hpp +++ b/lib/config/expression.hpp @@ -29,6 +29,7 @@ #include "base/configerror.hpp" #include "base/convert.hpp" #include +#include #include namespace icinga @@ -164,6 +165,28 @@ private: boost::shared_ptr m_Expression; }; +class I2_CONFIG_API FutureExpression : public Expression +{ +public: + FutureExpression(const boost::shared_future >& future) + : m_Future(future) + { } + +protected: + virtual Value DoEvaluate(VMFrame& frame, DebugHint *dhint) const + { + return m_Future.get()->DoEvaluate(frame, dhint); + } + + virtual const DebugInfo& GetDebugInfo(void) const + { + return m_Future.get()->GetDebugInfo(); + } + +private: + mutable boost::shared_future > m_Future; +}; + class I2_CONFIG_API LiteralExpression : public Expression { public: