Re: svn commit: r1908547 - /subversion/trunk/subversion/svnserve/svnserve.c

2024-01-31 Thread Jun Omae
On Tue, Jan 30, 2024 at 9:42 PM Daniel Sahlberg
 wrote:
>
> Den tis 30 jan. 2024 kl 01:39 skrev Jun Omae :
>>
>> On 2024/01/30 6:15, Daniel Sahlberg wrote:
>> > Good catch! How about:
>> >
>> > [[[
>> > Index: subversion/svnserve/svnserve.c
>> > ===
>> > --- subversion/svnserve/svnserve.c  (revision 1915424)
>> > +++ subversion/svnserve/svnserve.c  (working copy)
>> > @@ -574,7 +574,7 @@ accept_connection(connection_t **connection,
>> >  || APR_STATUS_IS_ECONNABORTED(status)
>> >  || APR_STATUS_IS_ECONNRESET(status));
>> >
>> > -  return status
>> > +  return status && !sigtermint_seen
>> > ? svn_error_wrap_apr(status, _("Can't accept client connection"))
>> > : SVN_NO_ERROR;
>> >  }
>> > ]]]
>>
>> The `sigtermint_seen` variable is not defined if sigaction is unavailable.
>> Instead, how about the following patch?
>>
>> [[[
>> Index: subversion/svnserve/svnserve.c
>> ===
>> --- subversion/svnserve/svnserve.c  (revision 1915466)
>> +++ subversion/svnserve/svnserve.c  (working copy)
>> @@ -574,9 +574,14 @@ accept_connection(connection_t **connection,
>>  || APR_STATUS_IS_ECONNABORTED(status)
>>  || APR_STATUS_IS_ECONNRESET(status));
>>
>> -  return status
>> -   ? svn_error_wrap_apr(status, _("Can't accept client connection"))
>> -   : SVN_NO_ERROR;
>> +  if (!status)
>> +return SVN_NO_ERROR;
>> +#if APR_HAVE_SIGACTION
>> +  else if (sigtermint_seen)
>> +return SVN_NO_ERROR;
>> +#endif
>> +  else
>> +return svn_error_wrap_apr(status, _("Can't accept client connection"));
>>  }
>>
>>  /* Add a reference to CONNECTION, i.e. keep it and it's pool valid unless
>> ]]]
>
>
> Oh, yes of course. That looks much better. Please commit!
>
> Kind regards,
> Daniel
>

Thanks for the reviewing! I just pushed the changes in r1915492.

-- 
Jun Omae  (大前 潤)


Re: [patch] publication for the fix of swig-py bug fixed in r1915316

2024-01-31 Thread Nathan Hartman
On Tue, Jan 30, 2024 at 10:42 PM Yasuhito FUTATSUKI 
wrote:

> Hi, I'd like to publish the fix of swig-py bug in r1915316, the bug
> which can cause crash, in the release note for Subversion 1.14.
>
> Here is a draft I wrote, but I have no confidence about it.
> Could anyoune please refine it?
> [[[
> Index: 1.14.html
> ===
> --- 1.14.html   (revision 1915486)
> +++ 1.14.html   (working copy)
> @@ -1494,6 +1494,37 @@
>
>   
>
> +
> +SWIG Python bindings crash on repos.replay() API (Subversion 1.14.3
> only)
> +   + title="Link to this section">¶
> +
> +
> +Subversion 1.14.3 may crash when using the SWIG Python bindings,
> +by using repos.replay() API for transactions which contain many
> +changes, or by using it repeatedly.
> +
> +This is one of regressions on the fix of
> +https://issues.apache.org/jira/browse/SVN-4753";>issue #4916,
> +https://issues.apache.org/jira/browse/SVN-4753";>issue
> #4917, and
> +https://issues.apache.org/jira/browse/SVN-4753";>issue #4918,
> +which are issues on Python reference counts.



The three URLs above should be changed to 4916, 4917, 4918, respectively.


+
> +The regressions are double release of the None object in
> +repos.replay() and leakage of reference release of the handler
> +object in repos.parse_fns3().  The former can cause crash
> +except on Python 3.12 (and later).



Slight change suggestion: "The regressions are double release of the
None object in repos.replay() and reference leakage of
the handler object in repos.parse_fns3().  The former can cause a
crash (except on Python 3.12 and later)."

(The suggestion above assumes I understand the regression correctly.)


+
> +This issue is fixed in
> +https://svn.apache.org/r1915316";>r1915316, and the patch
> +(not including additional tests) can be aquired by the command:
> +
> +svn diff -c 1915338
> https://svn.apache.org/repos/asf/subversion/branches/1.14.x/subversion/bindings/swig/python/libsvn_swig_py
> |\
> +  python3 -c "import re,sys;[print(re.sub(r'^(---|\+\+\+|Index:) ', r'\1
> ./subversion/bindings/swig/python/libsvn_swig_py/',l),end='') for l in
> sys.stdin]"
> +
> +
> + 
> +
>  
>  Ruby bindings require swig 3.0.9
> ]]]



Are the revision numbers correct above? (It says fixed in r1915316 but 'svn
diff' command uses r1915338.)

Hope this helps,
Nathan