Re: ra-test.exe deadlock condition
On 2/5/2017 20:57, Stefan wrote: > On 2/4/2017 12:41, Stefan Fuhrmann wrote: >> On 31.01.2017 10:09, Stefan wrote: >>> Hi, >>> >>> I've been looking at the cause of a deadlock when running ra-test.exe >>> with -fs-type=fsx (trunk version). >>> >>> The most important findings are summed up here atm [1]. >>> >>> The issue was discussed with brane and danielsh on IRC (thanks for your >>> time, once again). >>> >>> As far as my current understanding of the problem goes: the deadlock is >>> caused by the fact that the apr_terminate() function registered in >>> svn_cmdline_init() via the atexit-call is called after the termination >>> of the threads which were created as part of the calls to >>> apr_thread_pool_push() in svn_fs_x__batch_fsync_run(). >>> >>> This means that apr's thread counter (thd_cnt) is getting out of sync >>> (since the apr-function thread_pool_func() is not executed) and then >>> gets stuck in thread_pool_cleanup() (waiting for the already terminated >>> threads to be terminated). >>> >>> To me it looks like svnserve's main-function already contains a >>> safeguard against a corresponding issue, and calls >>> apr_thread_pool_destroy(threads) (or was this a completely different >>> scenario?). This however does not cover the threads created from >>> svn_fs_x__batch_fsync_run(). >>> >>> Talking to danielsh and brane it became apparent to me that the issue >>> might not be too obvious (in the end it might still be an issue on how I >>> build SVN and therefore cause the atexit-registered apr_terminate() >>> function to be called too late). It's also not fully clear to me at >>> which exact point (in regards to registerd atexit()-calls) threads of >>> the process are terminated if the process itself terminates. If indeed >>> atexit()-registered functions get called after the threads are forcibly >>> terminates (which to me it looks like it does atm) it might contradict >>> the C(89/99) standard - see[2] 7.20.4.2/7.20.4.3. On the other side this >>> thread on stackoverflow [3] suggests it's simply undefined (by the >>> standard) what comes first. >>> >>> As danielsh suggested, I'm planning to come up with a plain minimal >>> repro app only based on APR demonstrating the problem, so to make it >>> more obvious (and double check for myself) what the issue is about. >>> >>> Regards, >>> Stefan >>> >>> [1] http://www.luke1410.de:8090/browse/MAXSVN-94 >>> [2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf >>> [3] >>> https://stackoverflow.com/questions/39655868/what-does-the-posix-standard-say-about-thread-stacks-in-atexit-handlers-what >>> >> Hi Stefan, >> >> I had a look at the code and found a possibly related problem. >> If you are using DLLs, this might have affected you. >> >> It would be nice if you could try r1781657 and see whether it >> makes any difference in your case. >> >> -- Stefan^2. > Hi Stefan^2, > > I tested trunk r1781790 which also includes your follow-up commit > (r1781726). With that one the ra-test.exe test which previously > deadlocked passes now. However, test 60 (basic_test.py) deadlocks now > (svnmucc.exe seems to be the process which is being tested here). > > I'm planning to details of the underlying issue which I think has now > been traced down to the actual root-cause in a blog post most likely > tomorrow. That should explain the actual issue in full detail then. > Details are published now here: http://www.luke1410.de/blog/?p=95 Regards, Stefan smime.p7s Description: S/MIME Cryptographic Signature
Re: svn commit: r1782003 - /subversion/trunk/CHANGES
On 2/8/2017 11:15, Stefan Sperling wrote: > On Tue, Feb 07, 2017 at 11:00:41PM +0100, Stefan wrote: >> On 2/7/2017 15:57, s...@apache.org wrote: >>> * FSX: Fix an initialization / life time issue with batch-fsync (r1781657) >> Didn't that issue get in post SVN 1.9 and hence is a noop-change in >> regards to the changelog for SVN 1.9.5 -> SVN 1.10? >> >> Regards, >> Stefan > I don't know this part of the code well enough to judge that question myself > without additional context. I bet others do, and I hope they will get involved > in this discussion as well. But in case they don't, references would help. > Such as pointers to relevant revisions or a mailing list thread. > > In any case, the 1.10 section for CHANGES is work in progress and open to > collaboration! If you find something you believe is wrong in there, feel > free to just edit it right away. We can discuss any concerns post-commit :-) As discussed on IRC: As far as I can see, the code which introduced the issue was introduced in r1686899 which was post SVN 1.9. Hence, removed the changelog entry in r1782267. Stefan2, if I'm wrong and overlooked something, just drop me a line and I'll reinstate the entry (if nobody else did it by then). Regards, Stefan
Re: wish for new API or extended one
On 08.02.2017 15:03, Evgeny Kotkov wrote: Stefan Sperling writes: Ah, indeed. Thank you very much for this patch! Please commit it when you get a chance. Thanks, I committed the fix in r1782169. Stefan Kueng writes: But as I mentioned: without getting the options again with svn_client_conflict_tree_get_resolution_options, the strings are not updated. Stefan, could you please update to the HEAD revision and confirm that it fixes the issue you were seeing? Yes, that solves my problems. Thanks! Stefan -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest interface to (Sub)version control /_/ \_\ http://tortoisesvn.net
FINAL REMINDER: CFP for ApacheCon closes February 11th
Dear Apache Enthusiast, This is your FINAL reminder that the Call for Papers (CFP) for ApacheCon Miami is closing this weekend - February 11th. This is your final opportunity to submit a talk for consideration at this event. This year, we are running several mini conferences in conjunction with the main event, so if you're submitting for one of those events, please pay attention to the instructions below. Apache: Big Data * Event information: http://events.linuxfoundation.org/events/apache-big-data-north-america * CFP: http://events.linuxfoundation.org/events/apache-big-data-north-america/program/cfp Apache: IoT (Internet of Things) * Event Information: http://us.apacheiot.org/ * CFP - http://events.linuxfoundation.org/events/apachecon-north-america/program/cfp (Indicate 'IoT' in the Target Audience field) CloudStack Collaboration Conference * Event information: http://us.cloudstackcollab.org/ * CFP - http://events.linuxfoundation.org/events/apachecon-north-america/program/cfp (Indicate 'CloudStack' in the Target Audience field) FlexJS Summit * Event information - http://us.apacheflexjs.org/ * CFP - http://events.linuxfoundation.org/events/apachecon-north-america/program/cfp (Indicate 'Flex' in the Target Audience field) TomcatCon * Event information - https://tomcat.apache.org/conference.html * CFP - http://events.linuxfoundation.org/events/apachecon-north-america/program/cfp (Indicate 'Tomcat' in the Target Audience field) All other topics and projects * Event information - http://events.linuxfoundation.org/events/apachecon-north-america/program/about * CFP - http://events.linuxfoundation.org/events/apachecon-north-america/program/cfp Admission to any of these events also grants you access to all of the others. Thanks, and we look forward to seeing you in Miami! -- Rich Bowen VP Conferences, Apache Software Foundation rbo...@apache.org Twitter: @apachecon (You are receiving this email because you are subscribed to a dev@ or users@ list of some Apache Software Foundation project. If you do not wish to receive email from these lists any more, you must follow that list's unsubscription procedure. View the headers of this message for unsubscription instructions.)
Re: wish for new API or extended one
Stefan Sperling writes: > Ah, indeed. Thank you very much for this patch! > Please commit it when you get a chance. Thanks, I committed the fix in r1782169. Stefan Kueng writes: > But as I mentioned: without getting the options again with > svn_client_conflict_tree_get_resolution_options, the strings are not > updated. Stefan, could you please update to the HEAD revision and confirm that it fixes the issue you were seeing? Thanks, Evgeny Kotkov
Re: svn commit: r1780191 - /subversion/site/publish/docs/release-notes/1.10.html
On Wed, Feb 08, 2017 at 01:59:24AM +0100, Johan Corveleyn wrote: > On Tue, Feb 7, 2017 at 1:11 PM, Stefan Sperling wrote: > > On Tue, Feb 07, 2017 at 12:40:10PM +0100, Johan Corveleyn wrote: > >> BTW: why no "deleted an item" for local change (can't look at the code > >> / resolver options right now myself, so just wondering ...)? > > > > I've just added one case where the resolver has special-purpose code for > > that (code which already existed in 1.9): https://svn.apache.org/r1781991 > > This case is in the first row (incoming edit vs local delete/move), > > and updates children which were moved out of the directory before the > > directory itself was deleted. > > > > Apart from 'accept current working copy state' (aka postpone), there > > is no other case where the resolver code looks for a local change > > with reason code svn_wc_conflict_reason_deleted. > > It is quite possible that the resolver is missing a lot of other cases > > where this condition would apply! > > Okay, I did some table-reshuffling attempt in r1782093. Feel free to > change it further or to revert back if you think it's worse than > before. Thanks! Looks good to me :) > Concerning 'delete a directory', I don't fully understand what you > meant, so I left that resolution option with '...'. What I'd expect to > be possible, if I had locally deleted a directory or a file, is to > ignore any incoming change, and just accept the working copy situation > (i.e. deleted) -- not postponing, but just accepting the working copy > situation and mark it as resolved. The other interesting option would > probably be: revert my local deletion and apply the incoming change. I mixed something up: 'accept current working copy state' maps to 'mark resolved', not 'postpone'. So you could use the 'accept current working copy state' option in this case and it should do what you expect.
Re: svn commit: r1782003 - /subversion/trunk/CHANGES
On Tue, Feb 07, 2017 at 11:00:41PM +0100, Stefan wrote: > On 2/7/2017 15:57, s...@apache.org wrote: > > * FSX: Fix an initialization / life time issue with batch-fsync (r1781657) > > Didn't that issue get in post SVN 1.9 and hence is a noop-change in > regards to the changelog for SVN 1.9.5 -> SVN 1.10? > > Regards, > Stefan I don't know this part of the code well enough to judge that question myself without additional context. I bet others do, and I hope they will get involved in this discussion as well. But in case they don't, references would help. Such as pointers to relevant revisions or a mailing list thread. In any case, the 1.10 section for CHANGES is work in progress and open to collaboration! If you find something you believe is wrong in there, feel free to just edit it right away. We can discuss any concerns post-commit :-)
Re: wish for new API or extended one
On Wed, Feb 08, 2017 at 12:30:03AM +0300, Evgeny Kotkov wrote:
> I think that the actual problem here is that calling the
> svn_client_conflict_option_set_moved_to_repos_relpath() function
> does not update the description (and the target path in the description
> could have changed).
>
> In other words, probably we're looking at something along these lines:
Ah, indeed. Thank you very much for this patch!
Please commit it when you get a chance.
Thanks,
Stefan
> [[[
> Index: subversion/include/svn_client.h
> ===
> --- subversion/include/svn_client.h (revision 1782081)
> +++ subversion/include/svn_client.h (working copy)
> @@ -4495,6 +4495,7 @@ svn_error_t *
> svn_client_conflict_option_set_moved_to_repos_relpath(
>svn_client_conflict_option_t *option,
>int preferred_move_target_idx,
> + svn_client_ctx_t *ctx,
>apr_pool_t *scratch_pool);
>
> /**
> Index: subversion/libsvn_client/conflicts.c
> ===
> --- subversion/libsvn_client/conflicts.c (revision 1782081)
> +++ subversion/libsvn_client/conflicts.c (working copy)
> @@ -9414,6 +9414,7 @@ svn_error_t *
> svn_client_conflict_option_set_moved_to_repos_relpath(
>svn_client_conflict_option_t *option,
>int preferred_move_target_idx,
> + svn_client_ctx_t *ctx,
>apr_pool_t *scratch_pool)
> {
>svn_client_conflict_t *conflict = option->conflict;
> @@ -9466,6 +9467,14 @@ svn_client_conflict_option_set_moved_to_repos_relp
>if (strcmp(move_target_repos_relpath, repos_relpath) == 0)
> {
>details->move_target_repos_relpath = repos_relpath;
> + /* Update option description. */
> + SVN_ERR(describe_incoming_move_merge_conflict_option(
> +&option->description,
> +conflict, ctx,
> +details,
> +conflict->pool,
> +scratch_pool));
> +
>return SVN_NO_ERROR;
> }
> }
> Index: subversion/svn/conflict-callbacks.c
> ===
> --- subversion/svn/conflict-callbacks.c (revision 1782081)
> +++ subversion/svn/conflict-callbacks.c (working copy)
> @@ -1751,16 +1751,9 @@ handle_tree_conflict(svn_boolean_t *resolved,
>if (conflict_option)
> {
>SVN_ERR(svn_client_conflict_option_set_moved_to_repos_relpath(
> -conflict_option, preferred_move_target_idx,
> iterpool));
> +conflict_option, preferred_move_target_idx,
> +ctx, iterpool));
>
> - /* Update option description. */
> - SVN_ERR(build_tree_conflict_options(
> -&tree_conflict_options,
> -&possible_moved_to_repos_relpaths,
> -&possible_moved_to_abspaths,
> -NULL, conflict, ctx,
> -scratch_pool, scratch_pool));
> -
>/* Update conflict description. */
>SVN_ERR(svn_client_conflict_tree_get_description(
> &incoming_change_description,
> &local_change_description,
> ]]]
>
>
> Regards,
> Evgeny Kotkov
