Hi Bazsi, Thanks for the follow up. It makes sense to be trapping the config error as soon as possible, and as you said, users should be encouraged to have sensible configurations. Regards Anthony
On 11/1/2011 at 04:37 AM, in message <1320075450.22972.127.camel@bzorp>, Balazs Scheidler <bazsi@balabit.hu> wrote: Hi Anthony,
Sorry for not answering any sooner, this email has gone the same fate as Jakub's, therefore I didn't read it when it was posted.
Thanks for noticing this problem, tracking it down and posting a patch. I decided to use a different implementation, which uses the configuration parser framework to report problems and committed this patch:
commit b061328b3e0d3b466ac29ef4e1b3ee384b7130b0 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Mon Oct 31 16:33:32 2011 +0100
configuration: report duplicate configuration elements
Previously if a configuration element name was used twice, the 2nd would be used and some memory was leaked on every reload.
This patch changes that by reporting duplicate IDs as configuration errors.
A patch was posted by Anthony, but a different implementation was used instead.
Reported-By: anthony lineham <anthony.lineham@alliedtelesis.co.nz> Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
It reports duplicate config elements like this:
/------------ |Error parsing config, duplicate destination in etc/syslog-ng.conf at line 29, column 13: | |destination d_file { | ^^^^^^ | |syslog-ng documentation: http://www.balabit.com/support/documentation/?product=syslog-ng |mailing list: https://lists.balabit.hu/mailman/listinfo/syslog-ng \- -----------
This also means, that such problems will prevent syslog-ng to start up, but I'd say anyone using duplicate config elements should take notice. :)
The leak should also be fixed by my patch too.
Thanks again.
On Thu, 2011-10-13 at 09:47 +1300, anthony lineham wrote:
Hi Everyone,
I've recently found a memory leak in version 3.0.4 that occurs if the configuration file contains any duplicate entries for the following items: sources, destinations, filters, rewrites, parsers. The leak occurs when the configuration is loaded or reloaded due to a SIGHUP. The config items are stored in a hash table (hash is based on the name of the filter). It seems that in the hashing utility if a request is made to add an item with the same key as a previous item the old item is replaced by the new item (libglib2-2.24.1/glib/ghash.c, g_hash_table_insert()). This is where the memory gets lost. Syslog-ng is not notified that the item has been discarded.
I considered several possibilities to fix this issue. There are other modes for the hash utility where destroy functions can be setup to deal with the discarded items, but these are orientated around discarding the old item. I thought this was risky as the item may already be referenced by other items. I'm not sure if this case??? In the end I decided to check for the presence of a duplicate in the table before adding the new item. If an existing item is found I discard the new item.
While I realise 3.0.4 is quite old now and perhaps not supported, it looks like the same basic mechanism is still used in OSE 3.3.1, so I'm submitting my code changes for consideration. I haven't tried reproducing the issue on 3.3.1.
Any comments?