[dev.icinga.com #1879] idomod: change stacked memory allocation for broker_data IDO_MAX_BUFLEN

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/1879

Created by mfriedrich on 2011-09-08 14:38:24 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2011-09-22 18:29:32 +00:00)
Target Version: 1.5.1
Last Update: 2014-12-08 14:35:56 +00:00 (in Redmine)

Icinga Version: 1.10.0
OS Version: any

---

there's the problem with the default buffer on stack allocation with the change of IDO_MAX_BUFLEN from 16K to 64K in combination with a horrible mklivestatus client thread implementation, causing write_to_all_logs invoking a threaded neb callback to idomod broker_log_data, calling up a stacked temp_buffer with 50KB, while mklivestatus thread size is only 64 KB.

the allocation of memory for that buffer needs to be changed, not the size as is.

further information below (german only)

http://www.nagios-portal.org/wbb/index.php?page=Thread&postID=156721#post156721

hi,

gunnar und ich haben das heute mal eine runde debugged, und sind auf folgendes draufgekommen:

* livestatus spannt eine reihe von client threads auf
* wenn ein client GET irgendwas einen fehler wirft, dann wird was ins log geschrieben
* write_to_all_logs ist eine core funktion, die einen nebcallback initiiert
* dieser neb callback trifft ALLE neb module, also auch idomod, merlin, etc - was halt mitlaeuft
* sofern das neb module, in dem fall idomod, eine callback funktion registriert hat, wird es den livestatus log entry auch verarbeiten wollen
* das passiert im client thread, nicht im main thread
* idomod verwendet seit 16.3.2011 eine stacksize von 50KB, um mehr datendurchsatz zu erlauben
* die stacksize fuer livestatus threads ist per default 64KB, womit unter bestimmten umstaenden der stack voll ist und sich der core verabschiedet

einen fix fuer idomod wirds in form einer 1.5.1 geben, das ist bereits implementiert und laeuft gerade durch den test parcours.

dass es in diesem fall die idomod war, ist zufall, ein aehnliches szenario koennte man bspweise mit nagios, ndoutils und veraenderter threadsize in callback funktionen durchspielen. opsview wirds mit sicherheit auch treffen, von dort stammt der patch naemlich.

bis auf weiters kann man sich mit folgender hilfe aus diesem blogpost behelfen, der ein aehnliches problem mit vermutlich gleicher ursache beschreibt - http://blog.jensschanz.de/?p=1143


Quellcode

1
2
3
4
5
6



define module{
        module_name     mk-livestatus
        module_type     neb
        path            /usr/local/lib/mk-livestatus/livestatus.o
        args            /usr/local/icinga/var/rw/live thread_stack_size=131072
    }



dass allerdings livestatus als neb module alle regeln einer nicht threadsafe neb api missachtet, und mit seinen client threads munter funktionen aufruft, deren internals wiederum neb callbacks mit auswirkungen auf andere neb module verursachen, ist in unseren augen grob fahrlaessige programmierung.
es gibt derzeit keine moeglichkeit, derartige hacks im core korrekt abzufangen und zu unterbinden. demnach sollte sich jeder neb module developer selbst fragen, inwiewweit er/sie bereit ist, die stabilitaet des cores zu gefaehrden, indem funktionen verwendet werden, deren thread safety weder garantiert noch bewiesen ist.

lg
michi 

https://git.icinga.org/?p=icinga-core.git;a=blobdiff;f=module/idoutils/include/idomod.h;h=4c1fe3a0bbbf1adc16293d0655c26f99c45594ae;hp=3408ecb5cf1fdc1edb37f390634e137f9eee27b4;hb=15e7d23b5a7cf8dcd42882c612c23c33bf781a91;hpb=571c599be801eb77e67cf36eb93612b98b30c6e5

Changesets

2011-09-08 14:40:34 +00:00 by mfriedrich 4ccb136

idoutils: idomod: change stacked memory allocation for broker_data IDO_MAX_BUFLEN #1879

refs #1879

[icinga-migration]

Updated by mfriedrich on 2011-09-22 18:29:32 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

this was resolved during a 6h debug session and works fine now. the livestatus git holds a fix for themselves which wouldn't cause client threads to use non-threadsafe core functionality.

[icinga-migration]

Updated by mfriedrich on 2014-12-08 14:35:56 +00:00

• Project changed from 18 to Core, Classic UI, IDOUtils
• Category changed from 70 to IDOUtils
• Icinga Version set to 1
• OS Version set to any