There is a problem with apreq temp file cleanup on win32. For each
POST request with POST body greater than 256KB, TWO temp files of
exact same size get created in temp directory and they are not deleted
even after the request is handled. They get deleted when apache server
is stopped.

To isolate and test this I wrote a test Apache module (code given
below) and did a post of size greater than 256KB and traced the code.
The issue is apreq_file_mktemp calls apr_file_open to create the temp
file and registers that file with apreq_file_cleanup for deletion; at
the end of request, apreq_file_cleanup calls apr_file_remove on that
file. But till then, nobody calls apr_file_close on that file.
apr_file_remove will only mark the file for deletion and actual delete
happens only when all the open file handles are closed.(See apr doc
and DeleteFile msdn help).

Problem is worse as we run out of temp file names after a while. Also,
two temp files are created for each request, one at
apreq_filter_prefech calling apreq_brigade_concat(line 286) and then
at apreq_parse_multipart calling apreq_brigade_concat(line 595), which
makes the problem to be hit twice as fast.

Here is a patch to fix the issue. I've added the file handle to the
cleanup data and call apr_file_close on that handle in the
apreq_file_cleanup. This fixes the issue. Test module, apache
configuration and a test form is provided below.

Index: library/util.c
===================================================================
--- library/util.c      (revision 516021)
+++ library/util.c      (working copy)
@@ -775,12 +775,14 @@

struct cleanup_data {
    const char *fname;
+       apr_file_t* f;
    apr_pool_t *pool;
};

static apr_status_t apreq_file_cleanup(void *d)
{
    struct cleanup_data *data = d;
+    apr_file_close(data->f);
    return apr_file_remove(data->fname, data->pool);
}

@@ -834,8 +836,9 @@
    rc = apr_file_mktemp(fp, tmpl, flag, pool);

    if (rc == APR_SUCCESS) {
-        apr_file_name_get(&data->fname, *fp);
+        apr_file_name_get(&data->fname, *fp);           
        data->pool = pool;
+        data->f = *fp;
    }
    else {
        apr_pool_cleanup_kill(pool, data, apreq_file_cleanup);




The simple test module I wrote is given below.
#include <string>
using namespace std;
/* http specific headers */
#include "httpd.h"    
#include "http_protocol.h"
#include "http_config.h"
#include "http_log.h"
#include "http_request.h"
/* apr headers */
#define APR_WANT_STRFUNC
#include "apr_want.h"
#include "apr_strings.h"
#include "apr_lib.h"
#include "apr_general.h"
#include "util_filter.h"
#include "apr_buckets.h"
/* apreq headers */
#include "apreq.h"
#include "apreq_param.h"
#include "apreq_module_apache2.h"
#include "apreq_module.h"
#include "apreq_util.h"

extern "C"
{       
        static void register_hooks(apr_pool_t *p);
        static int post_handler(request_rec *r);
        module AP_MODULE_DECLARE_DATA post_module = {
                STANDARD20_MODULE_STUFF,
                NULL,   /* dir config creater */
                NULL,   /* dir merger --- default is to override */
                NULL,   /* server config */
                NULL,   /* merge server config */
                NULL,   /* command table */
                register_hooks  /* register hooks */
        };      
};

static void register_hooks(apr_pool_t *p)
{
        ap_hook_handler(post_handler, NULL, NULL, APR_HOOK_MIDDLE);
}

static int print_params(void *r, const char *key, const char *value)
{
        apreq_param_t* pParam = apreq_value_to_param(value);
        apr_file_t* fd = apreq_brigade_spoolfile(pParam->upload);
        string strFilename = "not spooled";
        if(fd != NULL)
        {
                apr_finfo_t finfo;
                apr_file_info_get(&finfo, APR_FINFO_NAME | APR_FINFO_SIZE |
APR_FINFO_NLINK, fd);
                strFilename = finfo.fname;
        }

        ap_rprintf((request_rec*)r,
"<arg><%s><upload>%s</upload></%s></arg>", key, strFilename.c_str(),
key);
        return 1; /* continue */
}

static int post_handler(request_rec *r)
{
        apreq_handle_t* pReq = apreq_handle_apache2(r);
        // Lets get the body
        const apr_table_t* pReqBody;
        apreq_body(pReq, &pReqBody);
        if(pReqBody == NULL)
                return DECLINED;
        const apr_table_t* pUploads = apreq_uploads(pReqBody, r->pool);
        apr_table_do(print_params, r, pUploads, NULL);
        return OK;
}

In apache config file, add the following lines:

LoadModule post_module modules/mod_post.so
<Location /post>
SetHandler post_handler
</Location>

Then, put this in a html file under DocumentRoot and invoke it and
upload a file greater than 256KB.
<html>
<body>
<form method="post" enctype="multipart/form-data" action="/post">
File to upload: <input name="upfile" type="file"><br>
Notes about the file: <input name="note" type="text"><br>
<br>
<input value="Press" type="submit"> to upload the file!
</form>
</body>
</html>

I would like to file a bug for this in apache bugzilla, but couldn't
find any apreq related bugs or apreq component there.

Let me know how I can help in any way to get this problem fixed.

Thanks,
--
Vinay Y S
http://vinaytech.blogspot.com

Reply via email to