You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Vinay Y S <vi...@gmail.com> on 2007/03/08 19:05:40 UTC

apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

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

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sat, 10 Mar 2007, Vinay Y S wrote:

> On 3/9/07, Joe Schaefer <jo...@sunstarsys.com> wrote:
>>     flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
>>     /* Win32 needs the following to remove temp files.
>>      * XXX: figure out why the APR_SHARELOCK flag works;
>>      * a grep through the httpd sources seems to indicate
>>      * it's only used in sdbm files??
>>     */
>> #ifdef WIN32
>>     flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
>> #endif
>>     rc = apr_file_mktemp(fp, tmpl, flag, pool);
>> 
>> Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
>> we should just remove that and see if it fixes the problem Vinay
>> is seeing.
>
> Yes, removing APR_FILE_NOCLEANUP does exactly the same thing as my
> patch. Rather, we can remove the whole #ifdef WIN32 as APR_SHARELOCK
> isn't used for anything in file_io on win32 today.

I'll have to look at this - there's a discussion at
   http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
of why this was introduced. I remember that this was
a frustrating issue, as something that worked on one
system in removing temp files didn't work on another
(at least between Steve and myself). What was committed
seemed to work "most" of the time for "most" systems,
at least at that time.

-- 
best regards,
Randy


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Vinay Y S <vi...@gmail.com>.
On 3/9/07, Joe Schaefer <jo...@sunstarsys.com> wrote:
>     flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
>     /* Win32 needs the following to remove temp files.
>      * XXX: figure out why the APR_SHARELOCK flag works;
>      * a grep through the httpd sources seems to indicate
>      * it's only used in sdbm files??
>     */
> #ifdef WIN32
>     flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
> #endif
>     rc = apr_file_mktemp(fp, tmpl, flag, pool);
>
> Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
> we should just remove that and see if it fixes the problem Vinay
> is seeing.

Yes, removing APR_FILE_NOCLEANUP does exactly the same thing as my
patch. Rather, we can remove the whole #ifdef WIN32 as APR_SHARELOCK
isn't used for anything in file_io on win32 today.

-- 
Vinay Y S

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 11 Mar 2007, Joe Schaefer wrote:

> Which Win32 tho?  98,ME,XP,XPPro,Vista,NT,etc?

For me, it's XP, and if I remember correctly,
that's what Steve was using.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:

> On Sun, 11 Mar 2007, Joe Schaefer wrote:
>
>> May I suggest that people start posting version numbers of
>> both libapr and operating system?  All we're doing now is
>> running around blindly tweaking crap that we really
>> shouldn't be tweaking in the first place.
>
> The problems described at
>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
> were on Win32

Which Win32 tho?  98,ME,XP,XPPro,Vista,NT,etc?

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 11 Mar 2007, Joe Schaefer wrote:

> May I suggest that people start posting version numbers of
> both libapr and operating system?  All we're doing now is
> running around blindly tweaking crap that we really
> shouldn't be tweaking in the first place.

The problems described at
    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
were on Win32 with httpd-2.2.2, based on the
libapreq2-2.08-RC4 sources. VC++ 6 was used to compile
things.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Mon, 12 Mar 2007, Vinay Y S wrote:

> On 3/12/07, Joe Schaefer <jo...@sunstarsys.com> wrote:
[ ... ] 
>> Here's a different suggestion to try:
[ ... ] 
>> In other words, delete the file *before* closing it.
>> 
> This shouldn't matter. My version is apreq2-2.0.8 with Apache
> 2.2.2/2.2.4 build with Visual Studio 2003 running on Windows XP SP2
> with all updates.

I'm not sure if this is relevant, but the problems we
found earlier with this is with VC++ 6, which must be
used for compatibility with ActivePerl.

> I don't know the perl code, but I would assume it would be doing all
> it's stuff(open/copy/closing the temp file) before either of the two
> cleanup functions are called above.
>
> So, if we apply the patch suggested by Joe or me(second patch) and fix
> the perl code which originally triggered all this discussion, all
> should be fine.

You're probably right about fixing the perl code; however,
this may lie somewhere in the Perl/mod_perl side of things.
And due to the nature of the failures, it's a hard thing
to track down.

Just in case it wasn't clear from the earlier messages, the
failures we found were only in the upload test of the perl 
glue; running it many (>15) times in rapid succession
resulted in occasional (and seemingly, randomly occuring)
stray temp files remaining. The problem seemed worse on
Steve's system compared to mine, although we were running
similar software versions (Windows XP, VC++ 6, httpd-2.2.2, 
and the latest perl-5.8 and mod_perl at the time). The
"fix" that was introduced seemed to help this problem,
which I agree doesn't make sense from the C side. However,
as one of the major consumers of libapreq2 in the Win32 
world are people running ActivePerl with similar setups
who install this via ppm, it's important to support that
side of things.

It may be that this was a problem outside of libapreq2;
since the time of the above failures, I've upgraded perl,
mod_perl, and Apache, and haven't encountered similar
failures with either your patch or Joe's. I would though
like to get Steve's experiences with this, as his setup
encountered more of the original failures.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Vinay Y S <vi...@gmail.com>.
On 3/12/07, Joe Schaefer <jo...@sunstarsys.com> wrote:
> > Ok, lets presume DeleteFile() is failing, and passing
> > that error along to apr_file_remove().  In apreq_file_cleaup
> > let's then try what cygwin does in its unlink implementation:
> > opening the file again (using the apr API) with the APR_DELONCLOSE
> > flag set, then immediately closing it.
>
> Here's a different suggestion to try:
>
> Index: library/util.c
> ===================================================================
> --- library/util.c      (revision 517052)
> +++ library/util.c      (working copy)
> @@ -816,10 +816,6 @@
>         return rc;
>
>     data = apr_palloc(pool, sizeof *data);
> -    /* cleanups are LIFO, so this one will run just after
> -       the cleanup set by mktemp */
> -    apr_pool_cleanup_register(pool, data,
> -                              apreq_file_cleanup, apreq_file_cleanup);
>
>     /* NO APR_DELONCLOSE! see comment above */
>     flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
> @@ -828,10 +824,11 @@
>      * a grep through the httpd sources seems to indicate
>      * it's only used in sdbm files??
>     */
> -#ifdef WIN32
> -    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
> -#endif
>     rc = apr_file_mktemp(fp, tmpl, flag, pool);
> +    /* cleanups are LIFO, so this one will run just before
> +       the cleanup set by mktemp */
> +    apr_pool_cleanup_register(pool, data,
> +                              apreq_file_cleanup, apreq_file_cleanup);
>
>     if (rc == APR_SUCCESS) {
>         apr_file_name_get(&data->fname, *fp);
>
>
> In other words, delete the file *before* closing it.
>
This shouldn't matter. My version is apreq2-2.0.8 with Apache
2.2.2/2.2.4 build with Visual Studio 2003 running on Windows XP SP2
with all updates.

Here's what I get to see with filemon(from sysinternals) when I run
with my second patch(removing only the #ifdef win32 part):
** first apr_file_mktemp->apr_file_open->CreateFile **
12:33:59.625 AM	httpd.exe:4032	IRP_MJ_CREATE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	Options: Create
Access: 0012019F

** then after all the reads and writes etc.. finally, apr's
file_cleanup->CloseHandle **
** presense of IRP_MJ_CLEANUP in here indicates that the filehandle
count is zero. **
12:35:29.281 AM	httpd.exe:4032	IRP_MJ_CLEANUP	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:35:29.281 AM	httpd.exe:4032	IRP_MJ_CREATE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	Options: Open
Access: 00120189
12:35:29.281 AM	httpd.exe:4032	IRP_MJ_SET_INFORMATION
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	FileBasicInformation
12:35:29.281 AM	System:4032	IRP_MJ_CLEANUP	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:35:29.281 AM	System:4032	IRP_MJ_CLOSE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:35:29.281 AM	httpd.exe:4032	IRP_MJ_CREATE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	Options: Open
Access: 00000180
12:35:29.281 AM	System:4032	IRP_MJ_CLEANUP	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:35:29.281 AM	System:4032	IRP_MJ_CLOSE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:35:29.281 AM	httpd.exe:4032	IRP_MJ_CLOSE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS


** After that apreq_file_cleanup->apr_file_remove->DeleteFile **
** DeleteFile does this: open the file of the given name, mark it for
deletion, close the file. **
12:38:10.218 AM	httpd.exe:4032	IRP_MJ_CREATE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	Options: Open
Access: 00010080
12:38:10.218 AM	httpd.exe:4032	IRP_MJ_SET_INFORMATION
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS	Delete
12:38:10.218 AM	httpd.exe:4032	IRP_MJ_CLEANUP	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS
12:38:10.218 AM	httpd.exe:4032	IRP_MJ_CLOSE
	C:\DOCUME~1\pi\LOCALS~1\Temp\apreqq4EH3d	SUCCESS


Notice that interchanging the order of above two cleanup functinons
has no change in the final result. You either close the
file(CloseHandle) and then mark the file for deletion(DeleteFile) or
mark the file for deletion and then close the file, it's all the same.
The file will go if there were no other file handles(say, from inside
perl content handler that ran just before) holding on to it.

I don't know the perl code, but I would assume it would be doing all
it's stuff(open/copy/closing the temp file) before either of the two
cleanup functions are called above.

So, if we apply the patch suggested by Joe or me(second patch) and fix
the perl code which originally triggered all this discussion, all
should be fine.

Thanks,
Vinay Y S

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:

> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
>
> [...]
>
>> apr_file_remove() on Win32 is calling DeleteFile(), with
>> unicode issues taken into consideration. So if apr_file_remove() is
>> failing, then DeleteFile() is failing.
>
> Ah yes, you are right.  I thought apr was defaulting
> to the unix version, which calls unlink(), but I see
> I'm mistaken.
>
> Ok, lets presume DeleteFile() is failing, and passing
> that error along to apr_file_remove().  In apreq_file_cleaup
> let's then try what cygwin does in its unlink implementation:
> opening the file again (using the apr API) with the APR_DELONCLOSE
> flag set, then immediately closing it.

Here's a different suggestion to try:

Index: library/util.c
===================================================================
--- library/util.c	(revision 517052)
+++ library/util.c	(working copy)
@@ -816,10 +816,6 @@
         return rc;
 
     data = apr_palloc(pool, sizeof *data);
-    /* cleanups are LIFO, so this one will run just after
-       the cleanup set by mktemp */
-    apr_pool_cleanup_register(pool, data,
-                              apreq_file_cleanup, apreq_file_cleanup);
 
     /* NO APR_DELONCLOSE! see comment above */
     flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
@@ -828,10 +824,11 @@
      * a grep through the httpd sources seems to indicate
      * it's only used in sdbm files??
     */
-#ifdef WIN32
-    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
-#endif
     rc = apr_file_mktemp(fp, tmpl, flag, pool);
+    /* cleanups are LIFO, so this one will run just before
+       the cleanup set by mktemp */
+    apr_pool_cleanup_register(pool, data,
+                              apreq_file_cleanup, apreq_file_cleanup);
 
     if (rc == APR_SUCCESS) {
         apr_file_name_get(&data->fname, *fp);


In other words, delete the file *before* closing it.

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:

[...]

> apr_file_remove() on Win32 is calling DeleteFile(), with
> unicode issues taken into consideration. So if apr_file_remove() is
> failing, then DeleteFile() is failing.

Ah yes, you are right.  I thought apr was defaulting
to the unix version, which calls unlink(), but I see
I'm mistaken.

Ok, lets presume DeleteFile() is failing, and passing
that error along to apr_file_remove().  In apreq_file_cleaup
let's then try what cygwin does in its unlink implementation:
opening the file again (using the apr API) with the APR_DELONCLOSE
flag set, then immediately closing it.

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 11 Mar 2007, Joe Schaefer wrote:

> So I think what's happening in the cases where the
> tempfiles aren't being removed is that the call to
> apr_file_remove is failing.  On windows, let's trap
> that error in apreq_file_cleanup and call DeleteFile()
> in that case.  If that fails return APR_EGENERAL.

>From what I can see from
   http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/open.c?revision=428317&view=markup
apr_file_remove() on Win32 is calling DeleteFile(), with
unicode issues taken into consideration. So if 
apr_file_remove() is failing, then DeleteFile() is
failing.

I've seen problems in the Perl world on Win32
unable to delete files because the file handle
is still in scope: for example,

   my $file = 'dummy.txt';
   open(my $fh, '>', $file);
   print $fh "Hello";
   unlink $file or warn $!;

works on Unix but fails on Win32 with a "permission
denied" error. The fix is to
   close($fh);
before unlinking the file. Vinay's original patch
with the addition of apr_file_close() I thought
was doing the analagous thing (I had originally
tried this, but had been inserting it too early
in the process).

By the way, as discussed at
  http://marc.theaimsgroup.com/?l=apreq-dev&m=115439946810955&w=2
I tried a simple C program that just uses the
code in apreq_file_mktemp(), with the registered
apreq_file_cleanup(), to open and close a temp
file - in this case, the temp files were cleaned
up with or without the

  #ifdef WIN32
     flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
  #endif

Since this problem of temp files only happens with
the Perl glue, it may be that there's some complex
interaction happening between apr and Perl.

> Also, get rid of this ifdef in apreq_file_mktemp:
>
> #ifdef WIN32
>    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
> #endif
>
> It's bogus, and IMO is only confusing the situation.

I agree, but, at least at the time it was added,
it definitely did help in the cleanup of temp
files. So it'd be nice to be able to replace
this with something more understandable.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:

> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:

[...]

>> Removing the APR_FILE_NOCLEANUP would, I think, bring
>> us back to the problem described at
>>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
>> for which this was introduced, in that some Win32 systems
>> have occasional stray temp files lingering around.
>
> May I suggest that people start posting version numbers of
> both libapr and operating system?  All we're doing now is 
> running around blindly tweaking crap that we really 
> shouldn't be tweaking in the first place.

Apologies for the tone of that,  I don't mean to sound so
discouraging.  Let me try to offer a suggestion for how
we might deal with this constructively.

The problem, as far as I can tell, stems from the fact
that apr uses the win32 api for some things, and the 
posix api for others.  On Windows those are two entirely
different subsystems, with varying levels of quality
depending on which version of Windows you are running.

So I think what's happening in the cases where the
tempfiles aren't being removed is that the call to
apr_file_remove is failing.  On windows, let's trap
that error in apreq_file_cleanup and call DeleteFile()
in that case.  If that fails return APR_EGENERAL.
Also, get rid of this ifdef in apreq_file_mktemp:

#ifdef WIN32
    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
#endif

It's bogus, and IMO is only confusing the situation.

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:

> On Sun, 11 Mar 2007, Vinay Y S wrote:
>
>> Actually, either my earlier patch which adds a apr_file_close in
>> apreq_file_cleanup or just removing the APR_FILE_NOCLEANUP from the
>> flags(patch attached) is enough. Both together isn't required. (but
>> it's safe as the cleanup handler installed by apr would get
>> uninstalled when you do apr_file_close). I favor doing just one of
>> them, preferably, removing the APR_FILE_NOCLEANUP flag. This makes the
>> WIN32 code identical to other platforms(where it's already working
>> fine, so no change for them). Patch for the same against the svn
>> source is given below. Please try this on win32 systems.
>
> Removing the APR_FILE_NOCLEANUP would, I think, bring
> us back to the problem described at
>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
> for which this was introduced, in that some Win32 systems
> have occasional stray temp files lingering around.

May I suggest that people start posting version numbers of
both libapr and operating system?  All we're doing now is 
running around blindly tweaking crap that we really 
shouldn't be tweaking in the first place.

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 11 Mar 2007, Vinay Y S wrote:

> Actually, either my earlier patch which adds a apr_file_close in
> apreq_file_cleanup or just removing the APR_FILE_NOCLEANUP from the
> flags(patch attached) is enough. Both together isn't required. (but
> it's safe as the cleanup handler installed by apr would get
> uninstalled when you do apr_file_close). I favor doing just one of
> them, preferably, removing the APR_FILE_NOCLEANUP flag. This makes the
> WIN32 code identical to other platforms(where it's already working
> fine, so no change for them). Patch for the same against the svn
> source is given below. Please try this on win32 systems.

Removing the APR_FILE_NOCLEANUP would, I think, bring
us back to the problem described at
    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
for which this was introduced, in that some Win32 systems
have occasional stray temp files lingering around.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Vinay Y S <vi...@gmail.com>.
On 3/11/07, Randy Kobes <ra...@theoryx5.uwinnipeg.ca> wrote:
> On Fri, 9 Mar 2007, Joe Schaefer wrote:
>
> > Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
> > we should just remove that and see if it fixes the problem Vinay
> > is seeing.
>
> Hi Steve, and all,
>  If you remember from
>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
> there was a problem with stray temp files in apreq
> remaining after uploads; we came up with a fix that
> worked most of the time, but Vinay has come up with
> something that appears better, and more understandable.
> The attached diff against library/util.c (in the svn
> sources) works for me in getting multiple invocations
> of the glue/perl/t/apreq/upload.t to pass; if you have
> time, could you try it out to see how it fares on
> your system? Thanks.

Actually, either my earlier patch which adds a apr_file_close in
apreq_file_cleanup or just removing the APR_FILE_NOCLEANUP from the
flags(patch attached) is enough. Both together isn't required. (but
it's safe as the cleanup handler installed by apr would get
uninstalled when you do apr_file_close). I favor doing just one of
them, preferably, removing the APR_FILE_NOCLEANUP flag. This makes the
WIN32 code identical to other platforms(where it's already working
fine, so no change for them). Patch for the same against the svn
source is given below. Please try this on win32 systems.


Thanks,
Vinay Y S


Index: library/util.c

===================================================================

--- library/util.c	(revision 516861)

+++ library/util.c	(working copy)

@@ -823,14 +823,6 @@


     /* NO APR_DELONCLOSE! see comment above */
     flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
-    /* Win32 needs the following to remove temp files.
-     * XXX: figure out why the APR_SHARELOCK flag works;
-     * a grep through the httpd sources seems to indicate
-     * it's only used in sdbm files??
-    */
-#ifdef WIN32
-    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
-#endif
     rc = apr_file_mktemp(fp, tmpl, flag, pool);

     if (rc == APR_SUCCESS) {

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:

> I've tried this just using the sleep() - the printf stuff
> doesn't work on Win32.

Hm. Try adding an \n or 2 to the error message.

> Unfortunately, I still see some apreqXXXXXX left over in the temp
> directory, very occasionally (this is with perl-5.8.8, Apache/2.2.2,
> mp2 2.000003). I tried a sleep(5), but that didn't help either.
>
> Sometimes, although it's not precisely
> correlated, an entry in the error log appears:
>   $param->upload_tempname($req): can't make spool bucket
>    at ...lib/APR/Request/Param.pm
> which comes from upload_tempname() in APR/Request/Param/Param.xs, which
> indicates that
> apreq_brigade_concat() has failed. However, sometimes
> a stray temp file remains without this error, and
> sometimes this error appears without a stray temp
> file remaining, so I'm not sure they're directly related.
>
> It's a frustrating problem - there doesn't seem to be
> much of a pattern; I can run the tests over and over
> again for 15 minutes without seeing any strays, but
> at other times they appear every few minutes. This machine
> isn't overly busy. The only common thing is that all of
> the temp files for me are copies of the perl.exe
> used in the upload test, and they're identical copies
> to the original.

I'd be happy to write this off as an OS bug if you are.
I wonder if it happens on NT?

-- 
Joe Schaefer

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 30 Mar 2007, Joe Schaefer wrote:

> I still believe the problem stems from mixing posix and win32 calls;
> but it's perl that mixes them, not apr.  In any case, we have evidence
> that our cleanup is failing, so we should include code that traps the
> error and tries to recover somehow.
>
> How about leaving the current upload.t tests alone, so they can
> generate tempfiles on Steve's box, and coming up with some cleanup
> code that, at least while we're trying to sort this out, does
> something like this:
>
> static apr_status_t apreq_file_cleanup(void *d)
> {
>    struct cleanup_data *data = d;
>    apr_status_t s = apr_file_remove(data->fname, data->pool);
>
> #ifdef WIN32
>
>    if (s != APR_SUCCESS) {
>        apr_file_t *stderr;
>        apr_pool_t *p;
>        const char fmt[] = "apr_file_remove failed with status %d on temp file %s. "
>            "Sleeping for 1 sec before retrying";
>
>        apr_pool_create_ex(&p, NULL, NULL, NULL);
>        apr_file_open_stderr(&stderr, p);
>        apr_file_printf(stderr, fmt, s, data->fname);
>
>        sleep(1); /* may need to #include <unistd.h> */
>
>        apr_pool_clear(p);
>        apr_pool_destroy(p);
>        s = apr_file_remove(data->fname, data->pool);
>    }
>
> #endif
>
>    return s;
> }

I've tried this just using the sleep() - the printf stuff
doesn't work on Win32. Unfortunately, I still see some
apreqXXXXXX left over in the temp directory, very 
occasionally (this is with perl-5.8.8, Apache/2.2.2,
mp2 2.000003). I tried a sleep(5), but that didn't
help either.

Sometimes, although it's not precisely
correlated, an entry in the error log appears:
   $param->upload_tempname($req): can't make spool bucket
    at ...lib/APR/Request/Param.pm
which comes from upload_tempname() in 
APR/Request/Param/Param.xs, which indicates that
apreq_brigade_concat() has failed. However, sometimes
a stray temp file remains without this error, and
sometimes this error appears without a stray temp
file remaining, so I'm not sure they're directly related.

It's a frustrating problem - there doesn't seem to be
much of a pattern; I can run the tests over and over
again for 15 minutes without seeing any strays, but
at other times they appear every few minutes. This machine
isn't overly busy. The only common thing is that all of
the temp files for me are copies of the perl.exe
used in the upload test, and they're identical copies
to the original.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Steve Hay <st...@uk.radan.com> writes:

> If I just apply the util.c change (i.e. drop the NOCLEANUP and
> SHARELOCK flags, but continue using perl.exe and httpd.exe for the
> upload tests) then all the tests still seem to pass anyway, but I do
> then see some stray "apreqXXXXXX" files left over in my temp directory
> from time to time 

I still believe the problem stems from mixing posix and win32 calls;
but it's perl that mixes them, not apr.  In any case, we have evidence
that our cleanup is failing, so we should include code that traps the
error and tries to recover somehow.

How about leaving the current upload.t tests alone, so they can
generate tempfiles on Steve's box, and coming up with some cleanup
code that, at least while we're trying to sort this out, does 
something like this:

static apr_status_t apreq_file_cleanup(void *d)
{
    struct cleanup_data *data = d;
    apr_status_t s = apr_file_remove(data->fname, data->pool);

#ifdef WIN32

    if (s != APR_SUCCESS) {
        apr_file_t *stderr;
        apr_pool_t *p;
        const char fmt[] = "apr_file_remove failed with status %d on temp file %s. "
            "Sleeping for 1 sec before retrying";
        
        apr_pool_create_ex(&p, NULL, NULL, NULL);
        apr_file_open_stderr(&stderr, p);
        apr_file_printf(stderr, fmt, s, data->fname);

        sleep(1); /* may need to #include <unistd.h> */

        apr_pool_clear(p);
        apr_pool_destroy(p);
        s = apr_file_remove(data->fname, data->pool);
    }

#endif

    return s;
}



-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Steve Hay <st...@uk.radan.com>.
Randy Kobes wrote:
> On Thu, 22 Mar 2007, Steve Hay wrote:
> 
>> Randy Kobes wrote:
>>> On Wed, 14 Mar 2007, Steve Hay wrote:
>>>
>>>> I tried your patch with the current svn version (revision 518242), 
>>>> but I'm still seeing intermittent failures (usually in tests 15, 16 
>>>> and/or 20) either when I run "nmake test" from the top-level,
> [ ... ]
>>>
>>> Perhaps just to narrow things down, could you try the
>>> attached C file (a VC++ Makefile is also attached)?
> [ ... ]
>> I ran each program with an argument of 500 for good measure.  All four 
>> programs successfully create and then delete a large number 
>> (presumably 500, I didn't count 'em!) of apreq* temp files.
>>
>> apr_temp and apr_temp_sh both deleted all those files about as quickly 
>> as they were made (i.e. very quickly), while apr_temp_nc and 
>> apr_temp_nc_sh both output loads of CLEANUP messages in the console 
>> quickly but then seemed to hang for nearly a minute, during which time 
>> loads of apreq* files were visible in the temp directory, and then 
>> eventually exited and all the files disappeared.
> 
> On my system (Windows XP, VC++ 6, Apache/2.2.2), there's
> essentially no difference in speed between the four
> programs in cleaning things up, and like you, all temp
> files are gone at the end. I'm afraid I'm stuck ...
> Since for you apr_temp/apr_temp_sh (fast) and
> apr_temp_nc/apr_temp_nc_sh (hangs a minute) pairwise
> behave similarly, that suggests APR_SHARELOCK isn't
> relevant (which is supported by it's apparent lack of
> relevance to this problem in the Apache sources). APR_FILE_NOCLEANUP 
> does seem to make a (negative)
> difference in speed, but perhaps
>    http://marc.info/?l=apr-dev&m=117448738223552&w=2
> is relevant here. In any case, the above results suggests
> that having APR_FILE_NOCLEANUP and APR_SHARELOCK both
> undefined would be better, which makes sense, but
> both apparently help in the Perl glue on Win32. Sigh ...
> 
> I'm wondering the following ... The failed upload
> tests (15, 16, and/or 20), for me, involve uploading
> the perl.exe binary. Is that the same for you? Might
> there be a problem with uploading a file which is
> a program that might be in use at the time? Does the
> attached patches against
>    apreq/library/util.c
> which removes the
>    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK
> line, and
>    apreq/glue/perl/t/apreq/upload.t
> which skips using perl.exe (and httpd.exe) for
> the upload tests on Win32 work for you?

I've tried running both "nmake test" and just the upload.t test many 
times over each and haven't yet seen either one fail with those two 
changes in place :-)

If I just apply the util.c change (i.e. drop the NOCLEANUP and SHARELOCK 
flags, but continue using perl.exe and httpd.exe for the upload tests) 
then all the tests still seem to pass anyway, but I do then see some 
stray "apreqXXXXXX" files left over in my temp directory from time to 
time (mostly when running "nmake test" rather than just the upload.t 
test), which didn't happen with the upload.t change applied as well.

That's really weird. I'm not sure I understand what's going on, but I 
think you might have cracked it at last!

-- 

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 22 Mar 2007, Steve Hay wrote:

> Randy Kobes wrote:
>> On Wed, 14 Mar 2007, Steve Hay wrote:
>> 
>>> I tried your patch with the current svn version (revision 518242), but I'm 
>>> still seeing intermittent failures (usually in tests 15, 16 and/or 20) 
>>> either when I run "nmake test" from the top-level,
[ ... ]
>> 
>> Perhaps just to narrow things down, could you try the
>> attached C file (a VC++ Makefile is also attached)?
[ ... ]
> I ran each program with an argument of 500 for good measure.  All four 
> programs successfully create and then delete a large number (presumably 500, 
> I didn't count 'em!) of apreq* temp files.
>
> apr_temp and apr_temp_sh both deleted all those files about as quickly as 
> they were made (i.e. very quickly), while apr_temp_nc and apr_temp_nc_sh both 
> output loads of CLEANUP messages in the console quickly but then seemed to 
> hang for nearly a minute, during which time loads of apreq* files were 
> visible in the temp directory, and then eventually exited and all the files 
> disappeared.

On my system (Windows XP, VC++ 6, Apache/2.2.2), there's
essentially no difference in speed between the four
programs in cleaning things up, and like you, all temp
files are gone at the end. I'm afraid I'm stuck ...
Since for you apr_temp/apr_temp_sh (fast) and
apr_temp_nc/apr_temp_nc_sh (hangs a minute) pairwise
behave similarly, that suggests APR_SHARELOCK isn't
relevant (which is supported by it's apparent lack of
relevance to this problem in the Apache sources). 
APR_FILE_NOCLEANUP does seem to make a (negative)
difference in speed, but perhaps
    http://marc.info/?l=apr-dev&m=117448738223552&w=2
is relevant here. In any case, the above results suggests
that having APR_FILE_NOCLEANUP and APR_SHARELOCK both
undefined would be better, which makes sense, but
both apparently help in the Perl glue on Win32. Sigh ...

I'm wondering the following ... The failed upload
tests (15, 16, and/or 20), for me, involve uploading
the perl.exe binary. Is that the same for you? Might
there be a problem with uploading a file which is
a program that might be in use at the time? Does the
attached patches against
    apreq/library/util.c
which removes the
    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK
line, and
    apreq/glue/perl/t/apreq/upload.t
which skips using perl.exe (and httpd.exe) for
the upload tests on Win32 work for you?

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Steve Hay <st...@uk.radan.com>.
Randy Kobes wrote:
> On Wed, 14 Mar 2007, Steve Hay wrote:
> 
>> I tried your patch with the current svn version (revision 518242), but 
>> I'm still seeing intermittent failures (usually in tests 15, 16 and/or 
>> 20) either when I run "nmake test" from the top-level, or when I run: 
>> perl -Iblib/arch -Iblib/lib t/TEST -verbose=1 t/apreq/upload.t from 
>> the glue/perl sub-directory :-( I'm running perl-5.8.8, apache-2.2.2 
>> and mod_perl-2.0.3 (RC3, I think).  Do I need to update anything there?
> 
> I don't think so - I ran the tests against essentially the
> same setup, and didn't see any failures when run a number
> of times.
> 
> Perhaps just to narrow things down, could you try the
> attached C file (a VC++ Makefile is also attached)?
> This uses the relevant parts of libapreq2 to create
> and cleanup a temp file; the Makefile produces 4 .exes:
>   apr_temp: don't enable APR_FILE_NOCLEANUP nor
>             APR_SHARELOCK
>   apr_temp_nc: enable only APR_FILE_NOCLEANUP
>   apr_temp_sh: enable only APR_SHARELOCK
>   apr_temp_nc_sh: enable both APR_FILE_NOCLEANUP and
>                   APR_SHARELOCK
> These are run, for example
>   apr_temp 20
> which will go in a loop and create, and then remove, 20
> temp files (with no arguments, the default is 10.
> 
> Are there any problems with the cleanup in any of these?
> If not, then it looks like the problem is somewhere
> within the Perl glue.

Apologies for the slow response.

I ran each program with an argument of 500 for good measure.  All four 
programs successfully create and then delete a large number (presumably 
500, I didn't count 'em!) of apreq* temp files.

apr_temp and apr_temp_sh both deleted all those files about as quickly 
as they were made (i.e. very quickly), while apr_temp_nc and 
apr_temp_nc_sh both output loads of CLEANUP messages in the console 
quickly but then seemed to hang for nearly a minute, during which time 
loads of apreq* files were visible in the temp directory, and then 
eventually exited and all the files disappeared.

-- 

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Wed, 14 Mar 2007, Steve Hay wrote:

> I tried your patch with the current svn version (revision 
> 518242), but I'm still seeing intermittent failures 
> (usually in tests 15, 16 and/or 20) either when I run 
> "nmake test" from the top-level, or when I run: perl 
> -Iblib/arch -Iblib/lib t/TEST -verbose=1 t/apreq/upload.t 
> from the glue/perl sub-directory :-( I'm running 
> perl-5.8.8, apache-2.2.2 and mod_perl-2.0.3 (RC3, I 
> think).  Do I need to update anything there?

I don't think so - I ran the tests against essentially the
same setup, and didn't see any failures when run a number
of times.

Perhaps just to narrow things down, could you try the
attached C file (a VC++ Makefile is also attached)?
This uses the relevant parts of libapreq2 to create
and cleanup a temp file; the Makefile produces 4 .exes:
   apr_temp: don't enable APR_FILE_NOCLEANUP nor
             APR_SHARELOCK
   apr_temp_nc: enable only APR_FILE_NOCLEANUP
   apr_temp_sh: enable only APR_SHARELOCK
   apr_temp_nc_sh: enable both APR_FILE_NOCLEANUP and
                   APR_SHARELOCK
These are run, for example
   apr_temp 20
which will go in a loop and create, and then remove, 20
temp files (with no arguments, the default is 10.

Are there any problems with the cleanup in any of these?
If not, then it looks like the problem is somewhere
within the Perl glue.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Steve Hay <st...@uk.radan.com> writes:

> I tried your patch with the current svn version (revision 518242), but I'm
> still seeing intermittent failures (usually in tests 15, 16 and/or 20) either
> when I run "nmake test" from the top-level, or when I run:
>
> perl -Iblib/arch -Iblib/lib t/TEST -verbose=1 t/apreq/upload.t
>
> from the glue/perl sub-directory :-(

Posting some diagnostic output might help.  Anything in the error logs?
Are you seeing any lingering tempfiles, and if so, are they
prefixed with apreqXXXX (ie coming from apreq_file_mktemp),
or are they from the test scripts?

Randy, I see a lot of stuff like

   unlink $file if -f $file;

in the upload.t tests, which might need to be changed to 

   unlink $file or die "Can't unlink $file: $!";

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Steve Hay <st...@uk.radan.com>.
Randy Kobes wrote:
> On Fri, 9 Mar 2007, Joe Schaefer wrote:
> 
>> Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
>> we should just remove that and see if it fixes the problem Vinay
>> is seeing.
> 
> Hi Steve, and all,
>  If you remember from
>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
> there was a problem with stray temp files in apreq
> remaining after uploads; we came up with a fix that
> worked most of the time, but Vinay has come up with
> something that appears better, and more understandable.
> The attached diff against library/util.c (in the svn
> sources) works for me in getting multiple invocations
> of the glue/perl/t/apreq/upload.t to pass; if you have
> time, could you try it out to see how it fares on
> your system? Thanks.

I tried your patch with the current svn version (revision 518242), but 
I'm still seeing intermittent failures (usually in tests 15, 16 and/or 
20) either when I run "nmake test" from the top-level, or when I run:

perl -Iblib/arch -Iblib/lib t/TEST -verbose=1 t/apreq/upload.t

from the glue/perl sub-directory :-(

I'm running perl-5.8.8, apache-2.2.2 and mod_perl-2.0.3 (RC3, I think). 
  Do I need to update anything there?

-- 


------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only. If you have received this message in error or there are any problems, please notify the sender immediately. The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd. The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Randy Kobes wrote:
> On Fri, 9 Mar 2007, Joe Schaefer wrote:
> 
>> Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
>> we should just remove that and see if it fixes the problem Vinay
>> is seeing.
> 
> Hi Steve, and all,
>  If you remember from
>    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
> there was a problem with stray temp files in apreq
> remaining after uploads; we came up with a fix that
> worked most of the time, but Vinay has come up with
> something that appears better, and more understandable.
> The attached diff against library/util.c (in the svn
> sources) works for me in getting multiple invocations
> of the glue/perl/t/apreq/upload.t to pass; if you have
> time, could you try it out to see how it fares on
> your system? Thanks.
Yeah, Steve / Randyk spent about 1-3 weeks discussing thinking about 
this b/c of test fails first mentioned by Issac during the 2.08 RC 
cycle.  The archives are in this list.


-- 
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/EC88A0BF 0DE5 C55C 6BF3 B235 2DAB  B89E 1324 9B4F EC88 A0BF

Work like you don't need the money,
love like you'll never get hurt,
and dance like nobody's watching.

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 9 Mar 2007, Joe Schaefer wrote:

> Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
> we should just remove that and see if it fixes the problem Vinay
> is seeing.

Hi Steve, and all,
  If you remember from
    http://marc.theaimsgroup.com/?t=115337629400001&r=1&w=2
there was a problem with stray temp files in apreq
remaining after uploads; we came up with a fix that
worked most of the time, but Vinay has come up with
something that appears better, and more understandable.
The attached diff against library/util.c (in the svn
sources) works for me in getting multiple invocations
of the glue/perl/t/apreq/upload.t to pass; if you have
time, could you try it out to see how it fares on
your system? Thanks.

-- 
best regards,
Randy

Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:

> "Vinay Y S" <vi...@gmail.com> writes:
>
>> 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.
>
> Thanks for the detailed report and patch!
> What version of apr are you using?  The reason I ask is 
> because whenever you open a file, normally a pool cleanup hook is
> registered to close it.

Ah, I see now. I should have looked at the source before opening
my mouth ;-):

    flag = APR_CREATE | APR_READ | APR_WRITE | APR_EXCL | APR_BINARY;
    /* Win32 needs the following to remove temp files.
     * XXX: figure out why the APR_SHARELOCK flag works;
     * a grep through the httpd sources seems to indicate
     * it's only used in sdbm files??
    */
#ifdef WIN32
    flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
#endif
    rc = apr_file_mktemp(fp, tmpl, flag, pool);

Randy, do you know why we use the APR_FILE_NOCLEANUP flag?  Maybe
we should just remove that and see if it fixes the problem Vinay
is seeing.

-- 
Joe Schaefer


Re: apreqXXXXXX temp files remain after processing uploads greater than 256kb. Further large upload fails

Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Vinay Y S" <vi...@gmail.com> writes:

> 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.

Thanks for the detailed report and patch!
What version of apr are you using?  The reason I ask is 
because whenever you open a file, normally a pool cleanup hook is
registered to close it.  apreq_file_mktemp() expects the pool cleanup
implicit in the apr_file_mktemp() call to close the file, so 
I'm wondering why that's not happening in your situation.

-- 
Joe Schaefer