Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
On Tue, 25 Jul 2006, Randy Kobes wrote: On Tue, 25 Jul 2006, Steve Hay wrote: Yes, that works for me! I tried the individual test and the whole test suite dozens of times over and didn't get a single failure. I'm not sure how it makes any difference, though, or exactly what it does. I searched the whole of my httpd-2.2.2 folder and only found one use of it (actually, of its new name, APR_FOPEN_SHARELOCK) relating to sdbm files. What am I missing? I'm baffled now, too - as far as I can see too, apr only uses APR_FOPEN_SHARELOCK in sdbm files, and neither mod_perl nor librapreq2 seems to use it. But it does make a difference - although I don't see as many failures as you do, without APR_FOPEN_SHARELOCK I definitely get temp files left over. Is the change safe, or does it introduce any security issues with temporary spool files being shared somehow? That I'm not sure of, especially now that I'm not sure what it's affecting ... I still haven't been able to track down why the use of APR_FOPEN_SHARELOCK works in cleaning up the temp files. I did try a simple C apr-based program that just opens a temp file in the same way as is done within apreq_file_mktemp(), with the registered apreq_file_cleanup(), writes some random text to it, and then closes it - in this the temp files were cleaned up with or without APR_FOPEN_SHARELOCK, and also with or without APR_FILE_NOCLEANUP. So something more complex is involved. Nevertheless, unless someone objects in the next day or so, I'd like to commit this change, as I think leaving temp files lying around is a worse problem. -- best regards, Randy
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
Nevertheless, unless someone objects in the next day or so, I'd like to commit this change, as I think leaving temp files lying around is a worse problem. No objection here :) -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F It takes a minute to have a crush on someone, an hour to like someone, and a day to love someone, but it takes a lifetime to forget someone...
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
Randy Kobes wrote: On Mon, 24 Jul 2006, Steve Hay wrote: Randy Kobes wrote: Also, just to verify that it is the stray temp files left over that are causing the problem, does it help if you change the APR_EXCL flag in the call to apr_file_mktemp on about line 832 of library/util.c to APR_TRUNCATE? Yep, that makes the errors go away: it didn't fail once in about two dozen runs. Does the following help? [...] With the APR_SHARELOCK flag, I don't see any temp files left over after about 50 runs of the upload.t test (without it, but still with APR_FILE_NOCLEANUP, I would have 2-3 left over after 50 runs). Yes, that works for me! I tried the individual test and the whole test suite dozens of times over and didn't get a single failure. I'm not sure how it makes any difference, though, or exactly what it does. I searched the whole of my httpd-2.2.2 folder and only found one use of it (actually, of its new name, APR_FOPEN_SHARELOCK) relating to sdbm files. What am I missing? Is the change safe, or does it introduce any security issues with temporary spool files being shared somehow? -- 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: [RELEASE CANDIDATE] libapreq2 2.08-RC4
On Tue, 25 Jul 2006, Steve Hay wrote: Yes, that works for me! I tried the individual test and the whole test suite dozens of times over and didn't get a single failure. I'm not sure how it makes any difference, though, or exactly what it does. I searched the whole of my httpd-2.2.2 folder and only found one use of it (actually, of its new name, APR_FOPEN_SHARELOCK) relating to sdbm files. What am I missing? I'm baffled now, too - as far as I can see too, apr only uses APR_FOPEN_SHARELOCK in sdbm files, and neither mod_perl nor librapreq2 seems to use it. But it does make a difference - although I don't see as many failures as you do, without APR_FOPEN_SHARELOCK I definitely get temp files left over. Is the change safe, or does it introduce any security issues with temporary spool files being shared somehow? That I'm not sure of, especially now that I'm not sure what it's affecting ... -- best regards, Randy
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
Steve Hay wrote: Sorry, but I'm still seeing quite a few failures. I started with a completely fresh build (with your patch applied) and the top-level nmake test failed a bunch of upload.t tests first time. (So it's not just running the test multiple times that causes the problem.) I then cd'd to perl/glue and ran just the upload.t test a dozen times over and it still failed 2 or 3 times. Is there an equivalent to 'strace' on 'that which you deem to call an OS' :) The 'open,access,stat*' might be useful ? -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F In all that I've done wrong I know I must have done something right to deserve a hug every morning and butterfly kisses at night.
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
Randy Kobes wrote: On Mon, 24 Jul 2006, Steve Hay wrote: Sorry, but I'm still seeing quite a few failures. I started with a completely fresh build (with your patch applied) and the top-level nmake test failed a bunch of upload.t tests first time. (So it's not just running the test multiple times that causes the problem.) I then cd'd to perl/glue and ran just the upload.t test a dozen times over and it still failed 2 or 3 times. Sigh :) ... One thing I tried (that didn't make a difference for me, but might for you) is in the call to apr_pool_cleanup_register( ...) on about line 829 of library/util.c, change the last arguments apreq_file_cleanup, apreq_file_cleanup to apreq_file_cleanup, apr_pool_cleanup_null Does that change anything? The cleanup in apr_file_open uses this latter form. Makes no difference for me either. I tried it with and without your previous APR_FILE_NOCLEANUP change, but I still see failures either way. Also, just to verify that it is the stray temp files left over that are causing the problem, does it help if you change the APR_EXCL flag in the call to apr_file_mktemp on about line 832 of library/util.c to APR_TRUNCATE? Yep, that makes the errors go away: it didn't fail once in about two dozen runs. -- 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: [RELEASE CANDIDATE] libapreq2 2.08-RC4
On Mon, 24 Jul 2006, Steve Hay wrote:
Randy Kobes wrote:
Also, just to verify that it is the stray temp files
left over that are causing the problem, does it help
if you change the APR_EXCL flag in the
call to apr_file_mktemp on about line 832 of library/util.c
to APR_TRUNCATE?
Yep, that makes the errors go away: it didn't fail once in about two dozen
runs.
Does the following help?
=
Index: util.c
===
--- util.c (revision 425268)
+++ 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);
@@ -829,9 +830,13 @@
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);
+/* 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 */
+#ifdef WIN32
+flag |= APR_FILE_NOCLEANUP | APR_SHARELOCK;
+#endif
+rc = apr_file_mktemp(fp, tmpl, flag, pool);
if (rc == APR_SUCCESS) {
apr_file_name_get(data-fname, *fp);
With the APR_SHARELOCK flag, I don't see any temp files
left over after about 50 runs of the upload.t test (without
it, but still with APR_FILE_NOCLEANUP, I would have 2-3
left over after 50 runs).
--
best regards,
Randy
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
Philip M. Gollucci wrote: Randy Kobes wrote: [Thu Jul 20 23:45:45 2006] [error] [client 127.0.0.1] (OS 80)The file exists. : apreq_brigade_concat failed; TempDir problem? which is coming from about line 288 of module/apache2/filter.c. The file exists message I think comes from the fact that, when the test fails, there's old temp files left over from a previous run. So perhaps the cleanup discussed in library/util.c at around line 797 is failing occasionally? You are absolutely correct. I can force duplicate on FBSD. I changed this (dropped the ) so I could predict the temp file name and yanked out all the tests not for tempname in t/apreq/upload.t rc = apr_filepath_merge(tmpl, path, apreq, APR_FILEPATH_NOTRELATIVE, pool); touch /tmp/apreq t/error_log [Fri Jul 21 04:02:42 2006] [error] [client 127.0.0.1] $param-upload_tempname($req): can't make spool bucket at /usr/home/pgollucci/dev/repos/asf/httpd/apreq/trunk/glue/perl/t/response/TestApReq/upload.pm line 33. Which means apr_pool_cleanup_register(pool, data, apreq_file_cleanup, apreq_file_cleanup); which calls apr_file_remove() which on Win32 is way out of my league :( I'm guessing this is failing, but silently. or less likely the cleanup is just not being run ? Question, are you both ASCII or UNICODE ? -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F In all that I've done wrong I know I must have done something right to deserve a hug every morning and butterfly kisses at night.
Re: [RELEASE CANDIDATE] libapreq2 2.08-RC4
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. FYI, http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/mktemp.c?r1=62405r2=62404pathrev=62405 The cleanup on Win32 was removed from apr here... I'll bet that's it! -- Philip M. Gollucci ([EMAIL PROTECTED]) 323.219.4708 Consultant / http://p6m7g8.net/Resume/resume.shtml Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F In all that I've done wrong I know I must have done something right to deserve a hug every morning and butterfly kisses at night.
