Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
On Sat, Jun 22, 2013 at 07:39:23PM -0400, Greg Stein wrote: The flag could be something like busted-proxy = on. When you start seeing blog posts like In order to checkout from EXAMPLE.COM, you must set busted-proxy=on in your .subversion configuration, and that gets back to EXAMPLE.COM ... you can bet they'll dig into the problem and fix it :-) We should use a more specific name, for the same reason that we don't add new meanings to --force.
Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
On Jun 23, 2013 4:54 AM, Daniel Shahaf danie...@apache.org wrote: On Sat, Jun 22, 2013 at 07:39:23PM -0400, Greg Stein wrote: The flag could be something like busted-proxy = on. When you start seeing blog posts like In order to checkout from EXAMPLE.COM, you must set busted-proxy=on in your .subversion configuration, and that gets back to EXAMPLE.COM ... you can bet they'll dig into the problem and fix it :-) We should use a more specific name, for the same reason that we don't add new meanings to --force. I think I disagree, as this is pretty specific already. force has zero semantic meaning/association. But in this case: yes, the proxy is busted from our view. We have to degrade to work with it. If you really want to push, then proxy-hates-chunks could work well. Our goal is to get those proxies to handle chunked requests. Politically correct terminology is not very goal-oriented. Throw in some bitchiness, I say. Cheers, -g
[PATCH] Crash in TSVN with directory property conflict during update (was Re: Another crash in ra_serf in 1.8.0)
Stefan, this issue is unrelated to ssl tunnelling or authentication, or more in general unrelated to ra_serf. The particular scenario here is a user updating a directory with both local property changes and incoming property changes. Svn then calls TSVN code to check if the user cancelled the action. Looking at the code, I noticed that the wrong baton is passed in the call to TSVN. The attached patch (to a 1.8.x wc) will fix that. I also noticed that svn command-line apparently does not use the cancel_baton, so this code path is never executed in our tests. Can you let us know if this patch fixes the issue for TSVN users? thanks, Lieven On Fri, Jun 21, 2013 at 9:26 PM, Stefan Küng tortoise...@gmail.com wrote: On 20.06.2013 23:05, Lieven Govaerts wrote: Looks like it's the authentication handling when setting up a SSL tunnel that's at fault here, at least I can easily reproduce it with an apache http proxy connetion to a https repo. The ssl tunnel is started by a CONNECT request created by serf. When the proxy requests credentials, serf will call back to the application. As the application doesn't know about this request, it doesn't get a valid baton either, so can't get baton-session ... That baton it gets is the ctx used by the ssltunnel code. Hm, have to think about how we can solve this. Not sure it can be done with the existing API. I'm not sure, but another crash report seems related to this: https://www.crash-server.com/Problem.aspx?ClientID=tsvnProblemID=26624 The stacktrace of this one: TortoiseProc.exe!SVN::cancel(void * baton=0x00df49062108) Line 1782 C++ libsvn_tsvn.dll!svn_wc__conflict_invoke_resolver(svn_wc__db_t * db=0x00df49d1eac0, const char * local_abspath=0x00df49063598, const svn_skel_t * conflict_skel=0x00df49063598, const apr_array_header_t * merge_options=0x, svn_error_t * (svn_wc_conflict_result_t * *, const svn_wc_conflict_description2_t *, void *, apr_pool_t *, apr_pool_t *) * resolver_func=0x07fe5cf570b0, void * resolver_baton=0x00df48e10238, svn_error_t * (void *) * cancel_func=0x07f74b709220, void * cancel_baton=0x00df48e10238, apr_pool_t * scratch_pool=0x00df49062108) Line 1937 C libsvn_tsvn.dll!close_directory(void * dir_baton=0x00df48e4c480, apr_pool_t * pool=0x00df48e34218) Line 2911 C libsvn_tsvn.dll!close_directory(void * dir_baton=0x00df48e30298, apr_pool_t * pool=0x00df48e304c8) Line 262C libsvn_tsvn.dll!close_dir(report_dir_t * dir=0x00df0259) Line 807 C libsvn_tsvn.dll!maybe_close_dir_chain(report_dir_t * dir=0x) Line 1394 C libsvn_tsvn.dll!end_report(svn_ra_serf__xml_parser_t * parser=0x00df0259, svn_ra_serf__dav_props_t name, apr_pool_t * scratch_pool=0x00df49d9c880) Line 2238C libsvn_tsvn.dll!end_xml(void * userData=0x00df49dd5610, const char * raw_name=0x00df48e42c18) Line 1314C libaprutil_tsvn.dll!doContent(XML_ParserStruct * parser=0x, int startTagLevel=0, const encoding * enc=0x57663180, const char * s=0x00df4909d39b, const char * end=0x00df4909ddc7, const char * * nextPtr=0x00df49dd5640) Line 2236 C libaprutil_tsvn.dll!contentProcessor(XML_ParserStruct * parser=0x00df48e442a8, const char * start=0x0003, const char * end=0x00df48e42418, const char * * endPtr=0x) Line 1817 C libaprutil_tsvn.dll!XML_ParseBuffer(XML_ParserStruct * parser=0x00df49dd5610, int len=0, int isFinal=0) Line 1482C libaprutil_tsvn.dll!XML_Parse(XML_ParserStruct * parser=0x, const char * s=0x00c8, int len=1238531864, int isFinal=1238541944) Line 1472 C libsvn_tsvn.dll!svn_ra_serf__handle_xml_parser(serf_request_t * request=0x00df49d27f18, serf_bucket_t * response=0x00df49d31c19, void * baton=0x0ad7, apr_pool_t * pool=0x) Line 1681 C libsvn_tsvn.dll!handle_response(serf_request_t * request=0x00df49d27f18, serf_bucket_t * response=0x00df49d2a678, svn_ra_serf__handler_t * handler=0x00df49d2a5d8, int * serf_status=0x, apr_pool_t * scratch_pool=0x00df49d2dbb8) Line 2060 C libsvn_tsvn.dll!handle_response_cb(serf_request_t * request=0x00df49d27f18, serf_bucket_t * response=0x00df49d2a5d8, void * baton=0x07fe, apr_pool_t * scratch_pool=0x) Line 2096C libsvn_tsvn.dll!handle_response(serf_request_t * request=0x, apr_pool_t * pool=0x) Line 872C libsvn_tsvn.dll!read_from_connection(serf_connection_t * conn=0x00df49d2dbb8) Line 995 C libsvn_tsvn.dll!serf__process_connection(serf_connection_t * conn=0x00df4668a318, short events=-22920) Line 1108 C libsvn_tsvn.dll!serf_event_trigger(serf_context_t * s=0x00df4668a318, void * serf_baton=0x00df49d2fc40, const apr_pollfd_t *
Re: Crashes in 1.8.0 test suite on Solaris Sparc (wrong alignment in cache_lookup())
On Thu, Jun 20, 2013 at 4:43 PM, Rainer Jung rainer.j...@kippdata.dewrote: Hi there, I built and tested svn 1.8.0 today on Solaris 0 Sparc and got lots of test failures due to core dumps. The first few dumps I inspected all showed a bus error in #0 0xfe660760 in cache_lookup (path=0x10fce06 /A/D/H/pi3, revision=3, cache=0x17c1820) at subversion/libsvn_fs_fs/tree.c:357 The code is: for (i = 0; i + 4 = path_len; i += 4) hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path + i); From similar problems with other software I expect this to be an alignment issue: the cast to apr_uint32_t* expects the pointer to point to an address divisible by four, but the string path can be located at any address in memory. In fact the cores show, that the address is not divisible by four. Sparc is especially picky about correct alignment, other platforms might have more relaxed rules. I guess you need to replace path by the next address divisible by 4 (and decrement path_len) accordingly) before running that loop. Regards, Rainer Due to the back-end-forth changes made on this function, I combined the patches into one small summary patch and and proposed it for backport (r1495806 of 1.8.x-1495063). Please review. -- Stefan^2.
Re: [PATCH] Crash in TSVN with directory property conflict during update (was Re: Another crash in ra_serf in 1.8.0)
On 23.06.2013 15:07, Lieven Govaerts wrote: Stefan, this issue is unrelated to ssl tunnelling or authentication, or more in general unrelated to ra_serf. The particular scenario here is a user updating a directory with both local property changes and incoming property changes. Svn then calls TSVN code to check if the user cancelled the action. Looking at the code, I noticed that the wrong baton is passed in the call to TSVN. The attached patch (to a 1.8.x wc) will fix that. I also noticed that svn command-line apparently does not use the cancel_baton, so this code path is never executed in our tests. Can you let us know if this patch fixes the issue for TSVN users? I've tested your patch and it seems to work. I could reproduce the crash using your described scenario. After applying the patch it doesn't crash anymore and the debugger also shows that the Cancel() method is called correctly. Stefan -- ___ oo // \\ De Chelonian Mobile (_,\/ \_/ \ TortoiseSVN \ \_/_\_/The coolest interface to (Sub)version control /_/ \_\ http://tortoisesvn.net
Re: [PATCH] Crash in TSVN with directory property conflict during update (was Re: Another crash in ra_serf in 1.8.0)
On Sun, Jun 23, 2013 at 4:29 PM, Stefan Küng tortoise...@gmail.com wrote: On 23.06.2013 15:07, Lieven Govaerts wrote: Stefan, this issue is unrelated to ssl tunnelling or authentication, or more in general unrelated to ra_serf. The particular scenario here is a user updating a directory with both local property changes and incoming property changes. Svn then calls TSVN code to check if the user cancelled the action. Looking at the code, I noticed that the wrong baton is passed in the call to TSVN. The attached patch (to a 1.8.x wc) will fix that. I also noticed that svn command-line apparently does not use the cancel_baton, so this code path is never executed in our tests. Can you let us know if this patch fixes the issue for TSVN users? I've tested your patch and it seems to work. I could reproduce the crash using your described scenario. After applying the patch it doesn't crash anymore and the debugger also shows that the Cancel() method is called correctly. Ok, committed to trunk in r1495850 and proposed for backport to 1.8.x. Lieven Stefan -- ___ oo // \\ De Chelonian Mobile (_,\/ \_/ \ TortoiseSVN \ \_/_\_/The coolest interface to (Sub)version control /_/ \_\ http://tortoisesvn.net
Re: [serf-dev] Re: ssl tunnel with basic authentication currently broken (was Re: Another crash in ra_serf in 1.8.0)
On Sun, Jun 23, 2013 at 3:20 AM, Greg Stein gst...@gmail.com wrote: On Sat, Jun 22, 2013 at 3:26 PM, Lieven Govaerts svn...@mobsol.be wrote: On Sat, Jun 22, 2013 at 7:32 PM, Lieven Govaerts svn...@mobsol.be wrote: Stefan, attached patch to serf 1.2.1 should solve this particular type of crash you reported. The patch is made against a serf 1.2.x working copy as follows: $ svn merge ^/trunk -c 1943,1944 Unfortunately the attached patch was not entirely correct, even though for svn it seems to work ok, it breaks the new ssltunnel unit test. Attached an updated patch. I'll probably do some more testing this weekend, and commit any improvements to serf trunk. So now: 1943, 1944, and 1946. I've reviewed the work and (by inspection) it looks great. r1948 also for Basic, then r1950 for Digest. I think this is it for ssltunnel authentication. The actual code changes can be merged to 1.2.x without problems, but the test suite changes will be more difficult, as not all earlier modifications on trunk were merged in 1.2.x. And with no API changes :-D Yeah. At the cost of a bit of extra complexity, something to cleanup for serf 2.0 then (it was already on the list). Thanks! -g Lieven
Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
On 6/22/13 1:22 AM, Lieven Govaerts wrote: Even better if we could cache this info somewhere automatically. So that the first time svn connects to a server ever it will use this 'slow' procedure, and then persists the results http/1.0 vs http/1.1 and chunked-encoding supported somewhere. Like we already do for passwords and ssl certificate validation. Will this work if you change where you are connecting from, e.g. at home one day and then from a Starbucks or maybe your work where you have different proxies in the path? Blair
Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
If you really want to push, then proxy-hates-chunks could work well. Oh man. Not proxy-blows-chunks? (SCNR.)
[svnbench] Revision: 1495899 compiled Jun 24 2013, 00:22:04 on x86_64-unknown-linux-gnu
1.7.0@1181106 vs. trunk@1495850 Started at Mon Jun 24 00:25:46 UTC 2013 *DISCLAIMER* - This tests only file://-URL access on a GNU/Linux VM. This is intended to measure changes in performance of the local working copy layer, *only*. These results are *not* generally true for everyone. Charts of this data are available at http://svn-qavm.apache.org/charts/ Averaged-total results across all runs: --- Compare trunk@1495850 to 1.7.0 Navg operation 36/90.53|-36.897 TOTAL RUN 2K/5300.78| -0.005 add 72/180.75| -0.218 checkout 288/720.63| -0.769 commit 36/90.75| -0.006 copy 36/90.79| -0.060 delete 180/450.16| -3.699 info 72/180.52| -1.037 merge 1K/5160.76| -0.003 mkdir 96/210.78| -0.002 propdel 26K/6K0.66| -0.003 proplist 27K/6K0.68| -0.004 propset 2K/5910.68| -0.004 ps 72/181.86| +0.009 resolve 72/180.81| -0.038 resolved 504/1260.68| -0.059 status 36/90.69| -0.336 switch 504/1260.76| -0.162 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1495850' is faster. 2/3 means: '1.7.0' has 2 timings on record, the other has 3.) Above totals split into separate dir-levelsxdir-spread runs: Compare trunk@1495850,5x5 to 1.7.0,5x5 Navg operation 12/30.53|-99.646 TOTAL RUN 1K/4560.79| -0.005 add 24/60.78| -0.513 checkout 96/240.65| -1.912 commit 12/30.72| -0.007 copy 12/30.83| -0.119 delete 60/150.15|-10.907 info 24/60.54| -2.577 merge 1K/4700.75| -0.003 mkdir 96/200.77| -0.002 propdel 25K/6K0.67| -0.003 proplist 25K/6K0.68| -0.004 propset 2K/5520.68| -0.004 ps 24/63.90| +0.025 resolve 24/60.81| -0.097 resolved 168/420.70| -0.136 status 12/30.73| -0.769 switch 168/420.83| -0.277 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1495850,5x5' is faster. 2/3 means: '1.7.0,5x5' has 2 timings on record, the other has 3.) Compare trunk@1495850,100x1 to 1.7.0,100x1 Navg operation 12/30.50| -8.739 TOTAL RUN 336/710.78| -0.004 add 24/60.54| -0.094 checkout 96/240.45| -0.301 commit 12/30.75| -0.006 copy 12/30.63| -0.046 delete 60/150.44| -0.163 info 24/60.34| -0.412 merge 168/460.83| -0.003 mkdir 1K/3370.59| -0.005 proplist 1K/2730.61| -0.006 propset 84/330.63| -0.005 ps 24/61.30| +0.003 resolve 24/60.88| -0.008 resolved 168/420.61| -0.031 status 12/30.49| -0.194 switch 168/420.45| -0.185 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1495850,100x1' is faster. 2/3 means: '1.7.0,100x1' has 2 timings on record, the other has 3.) Compare trunk@1495850,1x100 to 1.7.0,1x100 Navg operation 12/30.53| -2.305 TOTAL RUN 12/30.57| -0.025 add 24/60.54| -0.046 checkout 96/240.48| -0.092 commit 12/30.77| -0.005 copy 12/30.56| -0.015 delete 60/150.64| -0.028 info 24/60.38| -0.121 merge 444/1110.53| -0.006 proplist 504/1260.55| -0.006 propset 24/60.54| -0.006 ps 24/60.83| -0.002 resolve 24/60.59| -0.010 resolved 168/420.53| -0.010 status 12/30.51| -0.047 switch 168/420.58| -0.025 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1495850,1x100' is faster. 2/3 means: '1.7.0,1x100' has 2 timings on record, the other has 3.) More detail: Timings for 1.7.0,5x5 Nmin max avg operation (unit is seconds) 12 192.14 255.84 211.01 TOTAL RUN 1K0.012.210.02 add 240.025.152.31 checkout 961.07 17.295.43 commit 120.010.130.03 copy 120.610.960.72 delete 606.32 31.69 12.77 info 245.318.495.65 merge 1K0.010.040.01 mkdir 960.010.020.01 propdel 25K0.010.030.01 proplist 25K0.010.060.01 propset 2K0.010.040.01 ps 240.010.010.01 resolve 240.400.660.51 resolved 1680.170.960.45 status 122.713.022.81 switch 1680.214.981.66 update -- Timings for trunk@1495850,5x5 Nmin max avg operation (unit is seconds) 3 110.34 112.22 111.37 TOTAL RUN
Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
On Sun, Jun 23, 2013 at 7:35 PM, Peter Samuelson pe...@p12n.org wrote: If you really want to push, then proxy-hates-chunks could work well. Oh man. Not proxy-blows-chunks? (SCNR.) LOL! Actually, after an offlist email with Daniel, I would like to insist on just calling it busted-proxy. We don't need to be fine-grained on a config option that is only used for edge/broken cases. Consider the counter-example, where we have three proxy-related problems. We create *three* configuration options, document all three, and implement all three across ra_serf. And they are all edge cases. That is a lot of documentation and work for edge cases. And why do we need to solve proxy problem #2, and *avoid* hauling in solutions for #1 and #3? To optimize the user experience in a non-ideal situation? [by optimizing use of the (busted) proxy] I believe that the right answer is that we would take all three proxy issues and lump them all under busted-proxy. Even if the target proxy doesn't suffer issue #3, it doesn't really hurt to throw in a solution for that, even though the user is only experiencing issue #1. And shoot... let's say that we *do* find a serious issue with a proxy, which we don't want to haul in when a user is only experiencing lack-of-chunked-requests. Then we just change busted-proxy from on/off to on/off/horribly-wrong. Or maybe *that* proxy problem gets its own config option. Unless/until, then I really think a single catch-all is the best approach. To be concrete, we've suggested sending a second OPTIONS request to detect lack of support for chunked requests. Two years from now, we discover another proxy problem and instruct the user to turn on busted-proxy. Is it really a hardship for *that* user to have a second OPTIONS in their startup logic? Nah. The user will work to fix the proxy issue, or lobby for its solution. While that happens, they will suffer a bit of degradation under the overall busted-proxy flag. This isn't an area that we need to partition and discretely solve. Especially since it shouldn't exist in the first place. Cheers, -g
Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c
On Sun, Jun 23, 2013 at 3:56 PM, Blair Zajac bl...@orcaware.com wrote: On 6/22/13 1:22 AM, Lieven Govaerts wrote: Even better if we could cache this info somewhere automatically. So that the first time svn connects to a server ever it will use this 'slow' procedure, and then persists the results http/1.0 vs http/1.1 and chunked-encoding supported somewhere. Like we already do for passwords and ssl certificate validation. Will this work if you change where you are connecting from, e.g. at home one day and then from a Starbucks or maybe your work where you have different proxies in the path? Well... I don't think that we're talking (yet) about a cache. Just a config and an extra detection step. In your scenario, you'll suffer a slightly slower start cost (the extra OPTIONS), and in the other, a somewhat degraded experience (switching to C-L for all requests). But it should auto-switch as you move about. (assuming we don't try to cache, at this point) Cheers, -g
