#4 mod_statistics leaks file descriptors across Apache reloads

[Kevin Brown]

As it is right now, mod_statistics will open the specified statistics target file whenever its post-config function is called. This results in a file descriptor leak across Apache reloads:

[root@localhost apachestatistics]# grep -i stat /etc/httpd/conf/httpd.conf
# Statically compiled modules (those listed by `httpd -l') do not need
[root@localhost apachestatistics]# grep -i stat /etc/httpd/conf.d/
autoindex.conf  README          userdir.conf    welcome.conf    
[root@localhost apachestatistics]# grep -i stat /etc/httpd/conf.modules.d/0
00-base.conf        00-lua.conf         00-proxy.conf       01-cgi.conf         
00-dav.conf         00-mpm.conf         00-systemd.conf     02-statistics.conf  
[root@localhost apachestatistics]# cat /etc/httpd/conf.modules.d/02-statistics.conf 
LoadModule mod_statistics_module modules/mod_statistics.so

StatisticsFile /tmp/httpdStatistics
StatisticsRecordAlias On
StatisticsServerDisable Off
[root@localhost apachestatistics]# systemctl start httpd
[root@localhost apachestatistics]# systemctl status httpd | grep PID
 Main PID: 16557 (httpd)
[root@localhost apachestatistics]# lsof -p 16557 | grep /tmp/httpdStatistics
httpd   16557 root  mem    REG              253,0          51492909 /tmp/httpdStatistics (path inode=16777609)
httpd   16557 root    8u   REG              253,0   912637 51492909 /tmp/httpdStatistics
httpd   16557 root    9u   REG              253,0   912637 51492909 /tmp/httpdStatistics
[root@localhost apachestatistics]# systemctl reload httpd
[root@localhost apachestatistics]# lsof -p 16557 | grep /tmp/httpdStatistics
httpd   16557 root  mem    REG              253,0          51492909 /tmp/httpdStatistics (path inode=16777609)
httpd   16557 root    8u   REG              253,0   912637 51492909 /tmp/httpdStatistics
httpd   16557 root    9u   REG              253,0   912637 51492909 /tmp/httpdStatistics
httpd   16557 root   10u   REG              253,0   912637 51492909 /tmp/httpdStatistics

As you can see from the above, the target file has already been opened twice even immediately after starting Apache. This is because Apache will invoke the post-config function twice at start, once for checking the configuration and once for actual execution.

I have crafted a fix for this. Here's the diff:

diff --git a/mod_statistics/mod_statistics.c b/mod_statistics/mod_statistics.c
index 31bca66..5b39da9 100644
--- a/mod_statistics/mod_statistics.c
+++ b/mod_statistics/mod_statistics.c
@@ -97,6 +97,8 @@ static const command_rec    directives[] = {
     { NULL }
 };

+
+
 module AP_MODULE_DECLARE_DATA    mod_statistics_module = {
     STANDARD20_MODULE_STUFF,
     NULL,               /* Per-directory configuration handler */
@@ -136,6 +138,8 @@ static int statistics_post_config(apr_pool_t *pool, apr_pool_t *plog, apr_pool_t
     server_rec* myserver;
     statistics_conf* config;
     int i, clearfile = 0;
+    apr_file_t *stats_file = NULL;
+

     char* absfile;
     apr_finfo_t finfo;
@@ -144,6 +148,25 @@ static int statistics_post_config(apr_pool_t *pool, apr_pool_t *plog, apr_pool_t
     apr_file_t* server_stat_file_fd = NULL;
     apr_mmap_t* server_stat_file_mm = NULL;

+
+    ap_log_error(APLOG_MARK, APLOG_INFO, 0, server, "mod_statistics: in statistics_post_config");
+
+    // If we've already got an open file, then close it.  Doing this
+    // also ensures that we don't generate a file descriptor leak
+    // across reloads.
+    //
+    // Note that we later open the file again no matter what.  The
+    // reason for that is that even if we saved the previously opened
+    // file, the configured file path might have changed.  It's
+    // simpler to just open the file again no matter what, and just
+    // close what we opened previously, if it's there in the first
+    // place.
+    if (apr_pool_userdata_get((void **)&stats_file, "mod_statistics_file_pointer", server->process->pool) == APR_SUCCESS) {
+   if (stats_file != NULL) {
+       apr_file_close(stats_file);
+   }
+    }
+
     rc = APR_SUCCESS;

     config = ap_get_module_config( server->module_config, &mod_statistics_module);
@@ -178,6 +201,8 @@ static int statistics_post_config(apr_pool_t *pool, apr_pool_t *plog, apr_pool_t
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, server, "Unable to Create/Open File: %s - Errno: %d",absfile, rc );
         return 0;
     }
+    apr_pool_userdata_set(config->fd, "mod_statistics_file_pointer", NULL, server->process->pool);
+    ap_log_error(APLOG_MARK, APLOG_INFO, 0, server, "Stats file %s opened", absfile);

     if (clearfile == 1) {
         ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, server, "mod_statistics: Recreating file: %s ... ",absfile );
@@ -309,6 +334,7 @@ static int statistics_post_config(apr_pool_t *pool, apr_pool_t *plog, apr_pool_t
         myserver = myserver->next;
     } while ( myserver != NULL);

+    ap_log_error(APLOG_MARK, APLOG_INFO, 0, server, "mod_statistics: returning from statistics_post_config");

     return rc; 
 }

With the above fix in place, the statistics file is opened only once, even across reloads:

[root@localhost apachestatistics]# systemctl start httpd
[root@localhost apachestatistics]# systemctl status httpd | grep PID
 Main PID: 16778 (httpd)
[root@localhost apachestatistics]# lsof -p 16778 | grep /tmp/httpdStatistics
httpd   16778 root  mem    REG              253,0          51492907 /tmp/httpdStatistics (path inode=16777609)
httpd   16778 root    8u   REG              253,0   912637 51492907 /tmp/httpdStatistics
[root@localhost apachestatistics]# systemctl reload httpd
[root@localhost apachestatistics]# lsof -p 16778 | grep /tmp/httpdStatistics
httpd   16778 root  mem    REG              253,0          51492907 /tmp/httpdStatistics (path inode=16777609)
httpd   16778 root    8u   REG              253,0   912637 51492907 /tmp/httpdStatistics

Please incorporate the above fix, or something equivalent that addresses the issue.