[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Fix Win32 file handle leak in translate-test.exe?

From: <kfogel_at_collab.net>
Date: 2003-12-18 16:19:42 CET

Branko Čibej <brane@xbc.nu> writes:
> All ra_local tests pass on Windows with this patch. I'm testing ra_dav
> and ra_svn right now. If these tests pass, too (and I don't see any
> reason why they wouldn't), I'd like to include this patch in 0.35.0. I
> think the fact that it fixes a test failure is reason enough, and it's
> also minor enough to be safe.
>
> Votes, please?

It hasn't even been committed on trunk yet, has it? (I don't see it.)

Let's have it there, so people can easily review it before voting. I
know you posted it before, and I'm 99% sure it's the one I'm quoting
below, but it needs to go on trunk anyway, and once it's there, we
have a concrete revision to talk about.

-K

Brane's patch:

[[[
Fix a handle leak that caused translate-test to fail on Windows because
an open file cannot be removed there.

* subversion/libsvn_subr/subst.c (svn_subst_copy_and_translate): Use a
  subpool to open all files and streams and destroy it to make sure
  that all file handles are closed before returning.
]]]

Index: subst.c
===================================================================
--- subst.c (revision 8027)
+++ subst.c (working copy)
@@ -41,6 +41,7 @@
 #include "svn_io.h"
 #include "svn_hash.h"
 #include "svn_subst.h"
+#include "svn_pools.h"
 
 void
 svn_subst_eol_style_from_value (svn_subst_eol_style_t *style,
@@ -711,29 +712,35 @@
   svn_stream_t *src_stream, *dst_stream;
   apr_file_t *s = NULL, *d = NULL; /* init to null important for APR */
   svn_error_t *err;
+ apr_pool_t *subpool;
 
   /* The easy way out: no translation needed, just copy. */
   if (! (eol_str || keywords))
     return svn_io_copy_file (src, dst, FALSE, pool);
 
+ subpool = svn_pool_create (pool);
+
   /* Open source file. */
- SVN_ERR (svn_io_file_open (&s, src, APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT, pool));
+ err = svn_io_file_open (&s, src, APR_READ | APR_BUFFERED,
+ APR_OS_DEFAULT, subpool);
+ if (err)
+ goto error;
 
   /* For atomicity, we translate to a tmp file and
      then rename the tmp file over the real destination. */
 
- SVN_ERR (svn_io_open_unique_file (&d, &dst_tmp, dst,
- ".tmp", FALSE, pool));
+ err = svn_io_open_unique_file (&d, &dst_tmp, dst,
+ ".tmp", FALSE, subpool);
+ if (err)
+ goto error;
 
   /* Now convert our two open files into streams. */
- src_stream = svn_stream_from_aprfile (s, pool);
- dst_stream = svn_stream_from_aprfile (d, pool);
+ src_stream = svn_stream_from_aprfile (s, subpool);
+ dst_stream = svn_stream_from_aprfile (d, subpool);
 
   /* Translate src stream into dst stream. */
   err = svn_subst_translate_stream (src_stream, dst_stream,
                                     eol_str, repair, keywords, expand);
-
   if (err)
     goto error;
 
@@ -746,23 +753,25 @@
   if (err)
     goto error;
 
- err = svn_io_file_close (s, pool);
+ err = svn_io_file_close (s, subpool);
   if (err)
     goto error;
 
- err = svn_io_file_close (d, pool);
+ err = svn_io_file_close (d, subpool);
   if (err)
- goto error;
+ goto error;
 
   /* Now that dst_tmp contains the translated data, do the atomic rename. */
- err = svn_io_file_rename (dst_tmp, dst, pool);
+ err = svn_io_file_rename (dst_tmp, dst, subpool);
   if (err)
     goto error;
 
+ svn_pool_destroy (subpool);
   return SVN_NO_ERROR;
+
  error:
+ svn_pool_destroy (subpool); /* Make sure all files are closed first. */
   svn_error_clear (svn_io_remove_file (dst_tmp, pool));
-
   return err;
 }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Dec 18 17:09:20 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.