[Home]

Summary:ASTERISK-23383: Wrong sense test on stat return code causes unchanged config check to break with include files.
Reporter:David Woolley (davidw)Labels:
Date Opened:2014-02-27 06:51:46.000-0600Date Closed:2014-03-05 14:42:29.000-0600
Priority:MinorRegression?
Status:Closed/CompleteComponents:Core/Configuration
Versions:SVN 1.8.25.0 Frequency of
Occurrence		Constant
Related
Issues:
causesASTERISK-23683 #includes - wildcard character in a path more than one directory deep - results in no config parsing on module reload
Environment:CentOS, although not really environment specific.Attachments:
Description:Whilst trying to understand why an old version (1.6.1.0) was not treating configuration files as unchanged during a reload if there were include files, we discovered the equivalent of the following code, which clearly has the wrong sense test on the result of stat.  This quote has been taken from the trunk version, but it has remained unchanged since the 1.8 branch version, and differs only in the use of direct assignments before that.

In config_cache_attribute in main/config.c

{code}
if (!stat(configfile, &statbuf)) {
      cfmstat_clear(cfmtime);
} else {
      cfmstat_save(cfmtime, &statbuf);
}
{code}

Whilst I still haven't worked out why the including files cfmtime needs updating here at all, the test is clearly wrong, as it only uses the stat result when it is invalid!

The main consequence of this, other than wasted processing, as the result of doing full reloads on unchanged files, is that queue statistics get reset on a null reload.  However we are also investigating a possible race in the queue statistics update, which may or may not still be current.
Comments:By: David Woolley (davidw) 2014-02-27 07:09:45.818-0600

Note.  The gdb debugging, on the old version, that led us to this indicates that, at least for the top level configuration file, the mtime field does get set, but the first time the code processes an include file in the parsing mode, rather than change checking mode, and calls config_cache_attribue, the bad code clears out the value.  (It doesn't actually process includes in change checking mode, as the the top level file always has a zero cached mtime, at that point.)
By: David Woolley (davidw) 2014-02-27 08:43:43.610-0600

The anomoly seems to have been there since the beginning of ignoring unchanged files:

r79747 | tilghman | 2007-08-16 22:09:46 +0100 (Thu, 16 Aug 2007) | 2 lines

Don't reload a configuration file if nothing has changed.
By: Kinsey Moore (kmoore) 2014-03-05 14:42:29.151-0600

The fix to the stat test has been committed to 1.8, 11, 12, and trunk. Thanks for the report and for finding the issue!