Dear Maintainer,
I guess I found the issue here.
It looks like centengine is crashing with an unmodified
default configuration after 60 minutes running.
By changing "retention_update_interval" from 60 to 1 this could
be lowered to 1 minute.
Further it looks like having null pointers in "custom_variables"
member is not unusual.
But because 'dump::customvariables' receives the 'obj' by reference,
the compiler removes in an optimized build the null check,
I guess because it assumes that a reference has to be valid by definition.
Attached patch changes the argument by reference to a pointer.
A package build with that patch checks like expected for null and
leaves the function.
Upstream seems to have changed obj.custom_variables to a container type,
which might therefore not suffer from this problem anymore.
Kind regards,
Bernhard
(gdb) print obj.custom_variables
$3 = (customvariablesmember_struct *) 0x0
https://sources.debian.org/src/centreon-engine/18.10.0-4/src/retention/dump.cc/#L127
(gdb) list 121,133
121 std::ostream& dump::customvariables(
122 std::ostream& os,
123 customvariablesmember_struct const& obj) {
124 for (customvariablesmember const* member(&obj);
125 member;
126 member = member->next)
127 if (member->variable_name)
128 os << "_" << member->variable_name << "="
129 << member->has_been_modified << ","
130 << (member->variable_value ? member->variable_value : "")
131 << "\n";
132 return (os);
133 }
https://sources.debian.org/src/centreon-engine/18.10.0-4/src/retention/dump.cc/#L454
(gdb) list 389,455
389 std::ostream& dump::service(std::ostream& os, service_struct const&
obj) {
...
454 dump::customvariables(os, *obj.custom_variables);
455 os << "}\n";
(gdb) bt
#0 com::centreon::engine::retention::dump::customvariables (os=..., obj=...)
at ./src/retention/dump.cc:127
#1 0x0000559743899013 in com::centreon::engine::retention::dump::host (os=...,
obj=...) at ./src/retention/dump.cc:264
#2 0x000055974389a62b in com::centreon::engine::retention::dump::hosts
(os=...) at ./src/retention/dump.cc:278
#3 com::centreon::engine::retention::dump::save (path=...) at
./src/retention/dump.cc:359
#4 0x000055974386126b in _exec_event_retention_save (event=<optimized out>) at
./src/events/timed_event.cc:176
#5 0x0000559743860b5d in handle_timed_event (event=event@entry=0x559744b5ea90)
at ./src/events/timed_event.cc:752
#6 0x000055974385becb in com::centreon::engine::events::loop::_dispatching
(this=0x559744b396a0) at ./src/events/loop.cc:233
#7 0x000055974385c9e9 in com::centreon::engine::events::loop::run
(this=0x559744b396a0) at ./src/events/loop.cc:90
#8 0x0000559743784774 in main (argc=<optimized out>, argv=<optimized out>) at
./src/main.cc:410
Unmodified Debian: || With
patch applied:
...7c0 <+0>: push %r14 || ...530
<+0>: push %r14
...7c2 <+2>: lea 0x290a5(%rip),%r14 # 0x5597438c086e ||
...7c9 <+9>: push %r13 || ...532
<+2>: push %r13
...7cb <+11>: push %r12 || ...534
<+4>: push %r12
...7cd <+13>: push %rbp || ...536
<+6>: push %rbp
...7ce <+14>: mov %rdi,%rbp || ...537
<+7>: mov %rdi,%rbp
...7d1 <+17>: push %rbx || ...53a
<+10>: push %rbx
|| ...53b
<+11>: test %rsi,%rsi <<< null check
|| ...53e
<+14>: je 0x560648b46630 <...customvariables...+256> <<< jump
...7d2 <+18>: mov %rsi,%rbx || ...544
<+20>: mov %rsi,%rbx
...7d5 <+21>: jmpq 0x559743897868 <...customvariables...+168> ||
...
... ||
...
...868 <+168>: cmpq $0x0,(%rbx) <<< crash ||
...
... || ...630
<+256>: pop %rbx
|| ...631
<+257>: mov %rbp,%rax
|| ...634
<+260>: pop %rbp
|| ...635
<+261>: pop %r12
|| ...637
<+263>: pop %r13
...639
<+265>: pop %r14
...63b
<+267>: retq
Description: Change argument by reference to pointer to avoid crash
GCC seems to expect always valid references, therefore removes
in optimized builds null checks of references.
Author: Bernhard Übelacker <[email protected]>
Bug-Debian: https://bugs.debian.org/948491
Bug: https://github.com/centreon/centreon-engine/issues/338
Forwarded: no
Last-Update: 2020-01-10
--- centreon-engine-18.10.0.orig/inc/com/centreon/engine/retention/dump.hh
+++ centreon-engine-18.10.0/inc/com/centreon/engine/retention/dump.hh
@@ -39,7 +39,7 @@ namespace retention {
std::ostream& comments(std::ostream& os);
std::ostream& contact(std::ostream& os, contact_struct const& obj);
std::ostream& contacts(std::ostream& os);
- std::ostream& customvariables(std::ostream& os, customvariablesmember_struct const& obj);
+ std::ostream& customvariables(std::ostream& os, customvariablesmember_struct const* obj);
std::ostream& downtime(std::ostream& os, scheduled_downtime_struct const& obj);
std::ostream& downtimes(std::ostream& os);
std::ostream& header(std::ostream& os);
--- centreon-engine-18.10.0.orig/src/retention/dump.cc
+++ centreon-engine-18.10.0/src/retention/dump.cc
@@ -92,7 +92,7 @@ std::ostream& dump::contact(std::ostream
"modified_service_attributes=" << (obj.modified_service_attributes & ~config->retained_contact_service_attribute_mask()) << "\n"
"service_notification_period=" << (obj.service_notification_period ? obj.service_notification_period : "") << "\n"
"service_notifications_enabled=" << obj.service_notifications_enabled << "\n";
- dump::customvariables(os, *obj.custom_variables);
+ dump::customvariables(os, obj.custom_variables);
os << "}\n";
return (os);
}
@@ -120,8 +120,8 @@ std::ostream& dump::contacts(std::ostrea
*/
std::ostream& dump::customvariables(
std::ostream& os,
- customvariablesmember_struct const& obj) {
- for (customvariablesmember const* member(&obj);
+ customvariablesmember_struct const* obj) {
+ for (customvariablesmember const* member = obj;
member;
member = member->next)
if (member->variable_name)
@@ -261,7 +261,7 @@ std::ostream& dump::host(std::ostream& o
os << (x > 0 ? "," : "") << obj.state_history[(x + obj.state_history_index) % MAX_STATE_HISTORY_ENTRIES];
os << "\n";
- dump::customvariables(os, *obj.custom_variables);
+ dump::customvariables(os, obj.custom_variables);
os << "}\n";
return (os);
}
@@ -451,7 +451,7 @@ std::ostream& dump::service(std::ostream
os << (x > 0 ? "," : "") << obj.state_history[(x + obj.state_history_index) % MAX_STATE_HISTORY_ENTRIES];
os << "\n";
- dump::customvariables(os, *obj.custom_variables);
+ dump::customvariables(os, obj.custom_variables);
os << "}\n";
return (os);
}
