Skip to content
Commits on Source (2)
  • Aleksy Barcz's avatar
    Removed signal handlers from draw3 · 2275b41d
    Aleksy Barcz authored 
    + caused deadlocks on program exit, as functions called from signal handler were not async-signal-safe (see: man signal-safety). Amongst others, malloc and pthread_join are not safe.
+ OS will cleanup our threads and db queue anyway
+ saving defined user params on exit is no longer necessary as we save them on every edit action
+ because of all the above, we can safely leave signal handling and cleanup to standard OS behavior on Linux
    2275b41d
  • Aleksy Barcz's avatar
    libparnt: lock everything · 9ce9aa92
    Aleksy Barcz authored 
    + there is a lock introduced in "Add global parser synchronous access mutex", but still draw3 segfaults on init
+ segfaults are not reproducible using valgrind, and happen in AddPar, where curr points to a badly initialized structure
+ altogether this looks very much like a multithreading issue
+ so introduce a recursive_lock, locking every single function in libparnt
+ this is extremely ugly, but it works and it doesn't slow down program start remarkably
+ a proper fix would be removing all the libparnt code and writing a new, simple, parser for szarp.cfg
    9ce9aa92
Show whitespace changes
Inline Side-by-side
libSzarp/libparnt.cc
View file @ 9ce9aa92
......@@ -45,6 +45,8 @@
Par *globals = NULL;
Sections *sect = NULL;
static std::recursive_mutex libparnt_mutex;
namespace {
bool libpar_initted = false;
};
......@@ -61,6 +63,7 @@ void libpar_reset();
Sections *AddSection(const char *name)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Sections *nowa, *curr;
if (strcasecmp(name, "global") == 0)
......@@ -94,6 +97,7 @@ Sections *AddSection(const char *name)
void AddPar(Par ** list, const char *name, const char *content)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Par *nowa, *curr;
/* replace existing parameter content if param already exists */
......@@ -135,6 +139,7 @@ void AddPar(Par ** list, const char *name, const char *content)
void DeleteParList(Par ** list)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Par *curr, *nextcurr;
if (!(*list))
......@@ -152,6 +157,7 @@ void DeleteParList(Par ** list)
void DeleteSectList()
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Sections *curr, *nextcurr;
if (!sect)
......@@ -169,6 +175,7 @@ void DeleteSectList()
char *SeekPar(Par * list, const char *name)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Par *curr;
int found;
......@@ -191,6 +198,7 @@ char *SeekPar(Par * list, const char *name)
Sections *SeekSect(const char *name)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Sections *curr;
int found;
......@@ -215,12 +223,14 @@ Sections *SeekSect(const char *name)
#ifndef MINGW32
void libpar_init()
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_init_with_filename(NULL, 1);
}
//load 'szarp.cfg' from folder. Path to a directory must end with '/'
void libpar_init_from_folder(std::string folder_path)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
if(folder_path.empty())
return; //empty string
std::string config_name = CFGNAME;
......@@ -246,6 +256,7 @@ void libpar_init_from_folder(std::string folder_path)
*/
void libpar_read_cmdline(int *argc, char *argv[])
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
int i, j;
int l;
char *endptr;
......@@ -320,6 +331,7 @@ void libpar_read_cmdline(int *argc, char *argv[])
*/
void libpar_read_cmdline_w(int *argc, wchar_t *argv[])
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
int i, j;
int l;
wchar_t *endptr;
......@@ -386,6 +398,7 @@ void libpar_read_cmdline_w(int *argc, wchar_t *argv[])
int libpar_init_with_filename(const char *filename, int exit_on_error)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
if( libpar_initted )
libpar_reset();
libpar_initted = true;
......@@ -433,6 +446,7 @@ int libpar_init_with_filename(const char *filename, int exit_on_error)
void libpar_done()
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
DeleteParList(&globals);
DeleteParList(&command_line_pars);
DeleteSectList();
......@@ -441,6 +455,7 @@ void libpar_done()
void libpar_getkey(const char *section, const char *par, char **buf)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
Sections *s;
char *c;
......@@ -460,6 +475,7 @@ void libpar_getkey(const char *section, const char *par, char **buf)
void libpar_readpar(const char *section, const char *par, char *buf,
int size, int exit_on_error)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
char *c;
libpar_getkey(section, par, &c);
......@@ -477,6 +493,7 @@ void libpar_readpar(const char *section, const char *par, char *buf,
char *libpar_getpar(const char *section, const char *par, int exit_on_error)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
char *c;
libpar_getkey(section, par, &c);
......@@ -492,6 +509,7 @@ char *libpar_getpar(const char *section, const char *par, int exit_on_error)
}
void libpar_reset() {
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_initted = false;
int args_count = 0;
int i;
......@@ -543,23 +561,27 @@ void libpar_reset() {
#ifndef MINGW32
void libpar_reinit() {
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_reset();
libpar_init();
}
#endif
void libpar_hard_reset() {
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_reset();
parser_destroy();
}
void libpar_reinit_with_filename(const char *name, int exit_on_error) {
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_reset();
libpar_init_with_filename(name, exit_on_error);
}
void libpar_reinit_from_folder(const std::string& folder_path)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
libpar_reset();
libpar_init_from_folder(folder_path);
}
......@@ -568,6 +590,7 @@ void libpar_reinit_from_folder(const std::string& folder_path)
void libpar_setXenvironment(const char *programname)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
#define XENV "XENVIRONMENT"
char *c;
static char buf[1000];
......@@ -585,6 +608,7 @@ void libpar_setXenvironment(const char *programname)
void libpar_printfile(char *programname, char *printcmd, char *filename)
{
std::lock_guard<std::recursive_mutex> lguard(libparnt_mutex);
char *c, *ss;
libpar_getkey(programname, printcmd, &c);
......
wx/draw3/drawapp.cpp
View file @ 9ce9aa92
......@@ -151,12 +151,6 @@ extern void InitXmlResource();
bool read_only;
std::unique_ptr<FileLocker> m_instance;
void handler(int sig)
{
wxGetApp().OnExit();
_exit(0);
}
namespace {
int GL_ATTRIBUTES[] = {
WX_GL_RGBA,
......@@ -241,9 +235,6 @@ bool DrawApp::OnInit() {
SetProgName(_T("Draw 3"));
signal(SIGHUP, handler);
signal(SIGINT, handler);
signal(SIGTERM, handler);
if (m_just_print_version) {
std::cout << SZARP_VERSION << std::endl;
......
wx/draw3/include/drawapp.h
View file @ 9ce9aa92
......@@ -96,7 +96,7 @@ class DrawApp : public DrawGLApp
{
public:
/**
* Method called on application end and on sigint.
* Method called on application end.
*/
int OnExit();
......
wx/draw3/remarks.cpp
View file @ 9ce9aa92
......@@ -306,7 +306,7 @@ void RemarksHandler::GetConfigurationFromSzarpCfg() {
wxLogWarning(_T("defined __WXGTK__"));
//init libpar from folder, default: /opt/szarp/<base-name>
libpar_init_from_folder(std::string(m_config_manager->GetSzarpDir().mb_str()));
libpar_reinit_from_folder(std::string(m_config_manager->GetSzarpDir().mb_str()));
/* Check for remarks_server option in config file */
char *server = libpar_getpar("", "remarks_server", 0);
......