On Fri, 21 Jul 2006, Philip M. Gollucci wrote:

Philip M. Gollucci wrote:
Philip M. Gollucci wrote:
Randy Kobes wrote:
Which means
  apr_pool_cleanup_register(pool, data,
                              apreq_file_cleanup, apreq_file_cleanup);

Contrary to the comment in library/util.c
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);

   rc = apr_file_mktemp(fp, tmpl, /* NO APR_DELONCLOSE! see comment above */
                          APR_CREATE | APR_READ | APR_WRITE
                          | APR_EXCL | APR_BINARY, pool);

Win32 doesn't have mktemp, so thats not *strictly* true.

I think that refers to the cleanup set by apr_file_mktemp,
which in turn comes from apr_file_open (on Win32).

FYI,
http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/mktemp.c?r1=62405&r2=62404&pathrev=62405

The cleanup on Win32 was removed from apr here... I'll bet that's it!

The explicit cleanup was removed there, but replaced by
APR_DELONCLOSE (set by default), which if set would
delete the file on close. But, as the comment explains,
can't be used here ...

But I think you're on the right track, in that the
problem with the temp files sticking around (and
apparently causing the occasional errors when the
upload.t test is run multiple times) comes from the
cleanup set by apr_file_open. I tried this patch:

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

Index: util.c
===================================================================
--- util.c      (revision 424771)
+++ util.c      (working copy)
@@ -811,6 +811,7 @@
     apr_status_t rc;
     char *tmpl;
     struct cleanup_data *data;
+    apr_int32_t flag;

     if (path == NULL) {
         rc = apr_temp_dir_get(&path, pool);
@@ -828,11 +829,14 @@
        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;
+    /* needed on Win32 to cleanup temp files */
+#ifdef WIN32
+    flag |= APR_FILE_NOCLEANUP;
+#endif
+    rc = apr_file_mktemp(fp, tmpl, flag, pool);

- rc = apr_file_mktemp(fp, tmpl, /* NO APR_DELONCLOSE! see comment above */ - APR_CREATE | APR_READ | APR_WRITE
-                           | APR_EXCL | APR_BINARY, pool);
-
     if (rc == APR_SUCCESS) {
         apr_file_name_get(&data->fname, *fp);
         data->pool = pool;

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

which seems to help - when the upload.t test is run
multiple times, I get far fewer temp files left over
(although the occasional ones do stick around), and
even when they are there, I haven't seen any failures
yet. Steve, does this help you?

--
best regards,
Randy

Reply via email to