You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David James <ja...@gmail.com> on 2005/10/04 15:24:20 UTC

Re: svn commit: r16449 - trunk/build/generator

On 10/4/05, djames@tigris.org <dj...@tigris.org> wrote:
> Author: djames
> Date: Tue Oct  4 09:52:26 2005
> New Revision: 16449
>
> Modified:
>    trunk/build/generator/gen_make.py
>
> Log:
> Ensure that all python modules import libsvn._core before importing
> any other modules, so that the initialization code is run in the correct
> order (thus preventing segfaults). We can't implement this trick in SWIG,
> so we do it by postprocessing the SWIG output files.
>
> * build/generator/gen_make.py
>   (Generator.write): Update generated *.py files to import libsvn._core first.
>
>
>
> Modified: trunk/build/generator/gen_make.py
> Url: http://svn.collab.net/viewcvs/svn/trunk/build/generator/gen_make.py?rev=16449&p1=trunk/build/generator/gen_make.py&p2=trunk/build/generator/gen_make.py&r1=16448&r2=16449
> ==============================================================================
> --- trunk/build/generator/gen_make.py   (original)
> +++ trunk/build/generator/gen_make.py   Tue Oct  4 09:52:26 2005
> @@ -1,10 +1,10 @@
> -#
>  # gen_make.py -- generate makefiles and dependencies
>  #
>
>  import os
>  import sys
>  import string
> +import re
>
>  import gen_base
>  import generator.util.executable
> @@ -209,7 +209,20 @@
>          'cp -pf $(abs_srcdir)/%s/*.i ' % source_dir +
>          '$(abs_builddir)/%s; fi\n' % source_dir +
>          '\t$(SWIG) $(SWIG_INCLUDES) %s ' % opts +
> -        '-o $@ $(abs_builddir)/%s\n' % source +
> +        '-o $@ $(abs_builddir)/%s\n' % source
> +      )
> +      if objname.lang == "python":
> +        # Ensure that all python modules import libsvn._core before importing
> +        # any other modules, so that the initialization code is run in the
> +        # correct order. We can't implement this trick in SWIG, so we do it
> +        # by postprocessing the SWIG output files.
> +        pyfile = re.sub(r"(?:svn_)?(\w+)\.c$",r"\1.py", str(objname))
> +        self.ofile.write(
> +          '\tmv %s %s-swig ' % (pyfile, pyfile) +
> +          '&& (echo import _core | cat - %s-swig > %s) ' % (pyfile, pyfile) +
> +          '&& rm %s-swig\n' % (pyfile)
> +        )
> +      self.ofile.write(
>          'autogen-swig-%s: copy-swig-%s\n' % (short[objname.lang], objname) +
>          'copy-swig-%s: %s\n' % (objname, objname) +
>          '\t@if test $(abs_srcdir) != $(abs_builddir) -a ' +

Any suggestions for how we can port this fix to Windows?

Here's a test case:
  python -c "import svn.wc"

If the above test case doesn't work for you on Unix, run the following commands:
  # Regenerate .py files from scratch. Normally, makefiles do not automatically
  # regenerate output due to changes in the makefile itself, so we have to make
  # extraclean-swig-py here to take advantage of the changes in r16449
  svn up && ./autogen.sh && make extraclean-swig-py && make install
install-swig-py

Cheers,

David



--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 10/4/05, David James <ja...@gmail.com> wrote:

> I've figured out the problem: If we're using a threaded APR, and we
> import libsvn._wc before importing libsvn._core, the
> svn_swig_release_py_lock function will run before libsvn._core has had
> a chance to initialize APR. Does this fix look reasonable?

Why not just initialize APR in both places?  There's no reason you
can't initialize apr twice, as long as you terminate it twice, apr
internally will do the refcounting to make sure it only really shuts
itself down on the final terminate call.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> I've figured out the problem: If we're using a threaded APR, and we
> import libsvn._wc before importing libsvn._core, the
> svn_swig_release_py_lock function will run before libsvn._core has had
> a chance to initialize APR.

What is calling svn_swig_release_py_lock() that early ?

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/5/05, David James <ja...@gmail.com> wrote:
> On 10/5/05, Max Bowsher <ma...@ukf.net> wrote:
> > David James wrote:
> > > On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> > >> David James wrote:
> > >>> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> > >>>> David James wrote:
> > >>>>>
> > >>>>>   import _wc
> > >>>>>
> > >>>>> Oops! We imported _wc before importing _core. I'm not sure why
> > >>>>> importing _wc before _core leads to crashes on my machine now -- it
> > >>>>> used to work fine.
> > >>>>
> > >>>> Could you try using gdb to figure out where it crashes?
> > >>>>
> > >>>> It does not crash for me, nor would I expect it to, as no use of APR is
> > >>>> made in the _wc initialization function.
> > >>>>
> > >>>> In an "import svn.wc" situation, APR will be initialized when the
> > >>>> "import
> > >>>> core" statement in libsvn.wc executes, which follows immediately
> > >>>> (except
> > >>>> for some pure python swig internal definitions) after the "import _wc".
> > >>
> > >>> I've figured out the problem: If we're using a threaded APR, and we
> > >>> import libsvn._wc before importing libsvn._core, the
> > >>> svn_swig_release_py_lock function will run before libsvn._core has had
> > >>> a chance to initialize APR.
> > >>
> > >> I was do not have a threaded APR on my primary machine, so I've now
> > >> compiled Subversion on a Linux system which does have threaded APR:
> > >>
> > >> Results:
> > >>
> > >>>>> import svn.wc
> > >> Initializing libsvn._wc
> > >> Initializing libsvn._core
> > >> Initializing libsvn_swig_py threadkey
> > >>>>> # no crash
> > >>
> > >> (messages produced by some strategically placed fprintf(stderr, ...) in
> > >> the
> > >> C code)
> > >>
> > >> [Tests done with r16474]
> > >>
> > >> Thanks for reverting the earlier workaround.
> > >> Hope you have luck trying to trace the cause of the crash, because it
> > >> looks
> > >> like a threaded APR is not the cause.
> > >
> > > Here's some crash logs from python2.
> > > (I added a few helpful printf() statements.)
> > >
> > > Here's an example crash log:
> > >
> > >>>> import libsvn.wc
> > > Initializing libsvn._wc
> > > Initializing libsvn._core
> > > Calling apr_initialize() from libsvn._core
> > > Initializing libsvn_swig_py thread key
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0x40314567 in apr_pool_create_ex () from
> > > /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> > > #1  0x402fbe3b in svn_swig_py_release_py_lock ()
> > >     at
> > > /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> > > y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> > > (gdb) up
> > > #2  0x4021b4fd in _wrap_svn_swig_init_asp_dot_net_hack (self=0x0,
> > >     args=0x8105844) at subversion/bindings/swig/python/svn_wc.c:23044
> > > 23044           svn_swig_py_release_py_lock();
> > > (gdb) up
> > > #3  0x080cd299 in PyCFunction_Call ()
> > >
> > > Here's another crash log:
> > >>>> import svn.repos
> > > Initializing libsvn._core
> > > Calling apr_initialize() from libsvn._core
> > > Initializing libsvn_swig_py thread key
> > > (no debugging symbols found)...
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0x40300567 in apr_pool_create_ex () from
> > > /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> > > #1  0x402e7e3b in svn_swig_py_release_py_lock ()
> > >     at
> > > /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> > > y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> > > (gdb) up
> > > #2  0x40762d34 in _wrap_svn_pool_create (self=0x0, args=0x8105844)
> > >     at subversion/bindings/swig/python/core.c:7638
> > > 7638            svn_swig_py_release_py_lock();
> > > (gdb) up
> > > #3  0x080cd299 in PyCFunction_Call ()
> > > (gdb)
> > >
> > >
> > > Here's some test cases:
> > > NO CRASH: import libsvn.core; import libsvn.wc
> > > CRASH: import libsvn.wc
> > > NO CRASH: import libsvn.repos
> > > CRASH: import libsvn.repos; import libsvn.core; import libsvn.wc
> > > CRASH: import svn.repos
> > > NO CRASH: import libsvn.fs
> > > CRASH: import svn.fs
> > >
> > > NOTE:
> > > If you use the patch I attached earlier in this thread (which adjusts
> > > the location of the apr_initialize call) then everything works fine.
> > > I'm not sure why, though -- clearly, from the above output, you can
> > > see that APR was already initialized by the time we arrived in
> > > svn_swig_release_py_lock.
> >
> > How bizarre!
> > Perhaps running against an APR with debug symbols might bring something
> > useful in gdb?
> Max, are you using a static or a dynamic build of APR?
>
> I'm using a static build of APR. Does that mean that C/Python module
> (libsvn._core, libsvn._wc, etc) has a separate copy of the internal
> APR datastructures? If so, then it might be a good idea for us to
> separately initialize APR in each module.
>
> To test this theory, I added apr_initialize and apr_terminate calls to
> "svn_global.swg", so that each and every Python module separately
> initializes and terminates APR. This fixed the problem.
Here's a debug run which shows the problem: the static variable
global_pool is NULL in the context of libsvn_swig_py, because
apr_initialize was only called in libsvn._core, while libsvn_swig_py
was first loaded by libsvn._wc.

Python 2.2.2 (#1, Jan 30 2003, 21:26:22)
[GCC 2.96 20000731 (Red Hat Linux 7.3 2.96-112)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import libsvn.wc
apr_pool_create_ex(): global_pool=0, parent=0, allocator=81d0108
apr_pool_create_ex(): global_pool=81d4ba8, parent=0, allocator=0
apr_pool_create_ex(): global_pool=0, parent=0, allocator=0
(no debugging symbols found)...
Program received signal SIGSEGV, Segmentation fault.
0x40345b62 in apr_pool_create_ex (newpool=0x403e2110, parent=0x0, abort_fn=0,
    allocator=0x0) at apr_pools.c:806
806             allocator = parent->allocator;
(gdb) print global_pool
$1 = (struct apr_pool_t *) 0x0
(gdb) print parent
$2 = (struct apr_pool_t *) 0x0
(gdb) print allocator
$3 = (struct apr_allocator_t *) 0x0
(gdb) up
#1  0x4030074d in svn_swig_py_release_py_lock ()
    at /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_p
y/swigutil_py.c:77
77          apr_pool_create(&_saved_thread_pool, NULL);
(gdb) up
#2  0x40212a37 in _wrap_svn_swig_init_asp_dot_net_hack (self=0x0,
    args=0x8105844) at subversion/bindings/swig/python/svn_wc.c:23044
23044           svn_swig_py_release_py_lock();
(gdb) up
#3  0x080cd299 in PyCFunction_Call ()


--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> Yes, probably we should create a warning. I have reinstalled APR as a
> shared library and everything works fine.
>
> Still, I've found an easy way to get the bindings working with a
> static APR. The following patch makes the Subversion bindings with a
> static APR, on my system, because it initializes APR from inside the
> libsvn_swig_py library as opposed to from within a Python library. (We
> used to initialize APR from within libsvn_swig_py, until we changed
> this behaviour in r16382. That's why static APR builds always used to
> work for me.)

I don't fully understand why that works, but given the change is nice and 
simple, I have no objections to it.

A few small suggested tweaks, though.

> Create svn_swig_py_initialize API function for initializing the
> libsvn_swig_py library. This function initializes APR, and sets up
> atexit(apr_terminate). Followup to r16382.
>
> * swig/core.i:
>   Use svn_swig_py_initialize instead of apr_initialize and
>   apr_terminate.

The comment above says "these functions". Change to "this function", since 
there is now only one there.

> * swig/python/libsvn_swig_py/swigutil_py.h
>   (svn_swig_py_initialize): New.

Since you've put some error checking here, check the return value of atexit. 
No need to do anything complicated, I'm just thinking of something like 
this:
+    if (atexit(apr_terminate) != 0)
+      status = APR_EGENERAL;

> * swig/python/libsvn_swig_py/swigutil_py.c
>   (svn_swig_py_initialize): New.

Don't duplicate the documentation comment from the .h file here.


Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/5/05, Max Bowsher <ma...@ukf.net> wrote:
> David James wrote:
> > Max, are you using a static or a dynamic build of APR?
>
> Dynamic.
>
> > I'm using a static build of APR.
>
> Oh dear. There's the problem.
>
> > Does that mean that C/Python module
> > (libsvn._core, libsvn._wc, etc) has a separate copy of the internal
> > APR datastructures?
>
> Yes :-(
>
> > If so, then it might be a good idea for us to
> > separately initialize APR in each module.
>
> Issues with this:
> - atexit() slots may be a scarce finite resource - no more than 32 are
> guaranteed.
> - are there any issues with passing objects allocated by pools owned by one
> apr instance to another?
> - does libsvn_swig_py have an additional copy of apr, as well as all the
> modules?
> - it's worryingly inelegant
>
> I'm pretty surprised that the link succeeded at all, as I know that libtool
> is designed to outright refuse to link a shared library to a static library.
>
> Note also that we already require (though we do not enforce) that
> libsvn_swig_py be a shared library, with improper operation being the result
> if it is static (issue 2202). In view of this, I think we can reasonably
> require that apr also be a shared library.
>
> Perhaps we can create some configury to warn about this issue.
Yes, probably we should create a warning. I have reinstalled APR as a
shared library and everything works fine.

Still, I've found an easy way to get the bindings working with a
static APR. The following patch makes the Subversion bindings with a
static APR, on my system, because it initializes APR from inside the
libsvn_swig_py library as opposed to from within a Python library. (We
used to initialize APR from within libsvn_swig_py, until we changed
this behaviour in r16382. That's why static APR builds always used to
work for me.)

[[[

Create svn_swig_py_initialize API function for initializing the libsvn_swig_py
library. This function initializes APR, and sets up atexit(apr_terminate).
Followup to r16382.

* swig/core.i:
  Use svn_swig_py_initialize instead of apr_initialize and
  apr_terminate.

* swig/python/libsvn_swig_py/swigutil_py.h
  (svn_swig_py_initialize): New.

* swig/python/libsvn_swig_py/swigutil_py.c
  (svn_swig_py_initialize): New.

]]]


Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james

Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> Max, are you using a static or a dynamic build of APR?

Dynamic.

> I'm using a static build of APR.

Oh dear. There's the problem.

> Does that mean that C/Python module
> (libsvn._core, libsvn._wc, etc) has a separate copy of the internal
> APR datastructures?

Yes :-(

> If so, then it might be a good idea for us to
> separately initialize APR in each module.

Issues with this:
- atexit() slots may be a scarce finite resource - no more than 32 are 
guaranteed.
- are there any issues with passing objects allocated by pools owned by one 
apr instance to another?
- does libsvn_swig_py have an additional copy of apr, as well as all the 
modules?
- it's worryingly inelegant

I'm pretty surprised that the link succeeded at all, as I know that libtool 
is designed to outright refuse to link a shared library to a static library.

Note also that we already require (though we do not enforce) that 
libsvn_swig_py be a shared library, with improper operation being the result 
if it is static (issue 2202). In view of this, I think we can reasonably 
require that apr also be a shared library.

Perhaps we can create some configury to warn about this issue.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/5/05, Max Bowsher <ma...@ukf.net> wrote:
> David James wrote:
> > On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> >> David James wrote:
> >>> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> >>>> David James wrote:
> >>>>>
> >>>>>   import _wc
> >>>>>
> >>>>> Oops! We imported _wc before importing _core. I'm not sure why
> >>>>> importing _wc before _core leads to crashes on my machine now -- it
> >>>>> used to work fine.
> >>>>
> >>>> Could you try using gdb to figure out where it crashes?
> >>>>
> >>>> It does not crash for me, nor would I expect it to, as no use of APR is
> >>>> made in the _wc initialization function.
> >>>>
> >>>> In an "import svn.wc" situation, APR will be initialized when the
> >>>> "import
> >>>> core" statement in libsvn.wc executes, which follows immediately
> >>>> (except
> >>>> for some pure python swig internal definitions) after the "import _wc".
> >>
> >>> I've figured out the problem: If we're using a threaded APR, and we
> >>> import libsvn._wc before importing libsvn._core, the
> >>> svn_swig_release_py_lock function will run before libsvn._core has had
> >>> a chance to initialize APR.
> >>
> >> I was do not have a threaded APR on my primary machine, so I've now
> >> compiled Subversion on a Linux system which does have threaded APR:
> >>
> >> Results:
> >>
> >>>>> import svn.wc
> >> Initializing libsvn._wc
> >> Initializing libsvn._core
> >> Initializing libsvn_swig_py threadkey
> >>>>> # no crash
> >>
> >> (messages produced by some strategically placed fprintf(stderr, ...) in
> >> the
> >> C code)
> >>
> >> [Tests done with r16474]
> >>
> >> Thanks for reverting the earlier workaround.
> >> Hope you have luck trying to trace the cause of the crash, because it
> >> looks
> >> like a threaded APR is not the cause.
> >
> > Here's some crash logs from python2.
> > (I added a few helpful printf() statements.)
> >
> > Here's an example crash log:
> >
> >>>> import libsvn.wc
> > Initializing libsvn._wc
> > Initializing libsvn._core
> > Calling apr_initialize() from libsvn._core
> > Initializing libsvn_swig_py thread key
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x40314567 in apr_pool_create_ex () from
> > /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> > #1  0x402fbe3b in svn_swig_py_release_py_lock ()
> >     at
> > /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> > y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> > (gdb) up
> > #2  0x4021b4fd in _wrap_svn_swig_init_asp_dot_net_hack (self=0x0,
> >     args=0x8105844) at subversion/bindings/swig/python/svn_wc.c:23044
> > 23044           svn_swig_py_release_py_lock();
> > (gdb) up
> > #3  0x080cd299 in PyCFunction_Call ()
> >
> > Here's another crash log:
> >>>> import svn.repos
> > Initializing libsvn._core
> > Calling apr_initialize() from libsvn._core
> > Initializing libsvn_swig_py thread key
> > (no debugging symbols found)...
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x40300567 in apr_pool_create_ex () from
> > /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> > #1  0x402e7e3b in svn_swig_py_release_py_lock ()
> >     at
> > /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> > y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> > (gdb) up
> > #2  0x40762d34 in _wrap_svn_pool_create (self=0x0, args=0x8105844)
> >     at subversion/bindings/swig/python/core.c:7638
> > 7638            svn_swig_py_release_py_lock();
> > (gdb) up
> > #3  0x080cd299 in PyCFunction_Call ()
> > (gdb)
> >
> >
> > Here's some test cases:
> > NO CRASH: import libsvn.core; import libsvn.wc
> > CRASH: import libsvn.wc
> > NO CRASH: import libsvn.repos
> > CRASH: import libsvn.repos; import libsvn.core; import libsvn.wc
> > CRASH: import svn.repos
> > NO CRASH: import libsvn.fs
> > CRASH: import svn.fs
> >
> > NOTE:
> > If you use the patch I attached earlier in this thread (which adjusts
> > the location of the apr_initialize call) then everything works fine.
> > I'm not sure why, though -- clearly, from the above output, you can
> > see that APR was already initialized by the time we arrived in
> > svn_swig_release_py_lock.
>
> How bizarre!
> Perhaps running against an APR with debug symbols might bring something
> useful in gdb?
Max, are you using a static or a dynamic build of APR?

I'm using a static build of APR. Does that mean that C/Python module
(libsvn._core, libsvn._wc, etc) has a separate copy of the internal
APR datastructures? If so, then it might be a good idea for us to
separately initialize APR in each module.

To test this theory, I added apr_initialize and apr_terminate calls to
"svn_global.swg", so that each and every Python module separately
initializes and terminates APR. This fixed the problem.

Cheers,

David


--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
>> David James wrote:
>>> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
>>>> David James wrote:
>>>>>
>>>>>   import _wc
>>>>>
>>>>> Oops! We imported _wc before importing _core. I'm not sure why
>>>>> importing _wc before _core leads to crashes on my machine now -- it
>>>>> used to work fine.
>>>>
>>>> Could you try using gdb to figure out where it crashes?
>>>>
>>>> It does not crash for me, nor would I expect it to, as no use of APR is
>>>> made in the _wc initialization function.
>>>>
>>>> In an "import svn.wc" situation, APR will be initialized when the 
>>>> "import
>>>> core" statement in libsvn.wc executes, which follows immediately 
>>>> (except
>>>> for some pure python swig internal definitions) after the "import _wc".
>>
>>> I've figured out the problem: If we're using a threaded APR, and we
>>> import libsvn._wc before importing libsvn._core, the
>>> svn_swig_release_py_lock function will run before libsvn._core has had
>>> a chance to initialize APR.
>>
>> I was do not have a threaded APR on my primary machine, so I've now
>> compiled Subversion on a Linux system which does have threaded APR:
>>
>> Results:
>>
>>>>> import svn.wc
>> Initializing libsvn._wc
>> Initializing libsvn._core
>> Initializing libsvn_swig_py threadkey
>>>>> # no crash
>>
>> (messages produced by some strategically placed fprintf(stderr, ...) in 
>> the
>> C code)
>>
>> [Tests done with r16474]
>>
>> Thanks for reverting the earlier workaround.
>> Hope you have luck trying to trace the cause of the crash, because it 
>> looks
>> like a threaded APR is not the cause.
>
> Here's some crash logs from python2.
> (I added a few helpful printf() statements.)
>
> Here's an example crash log:
>
>>>> import libsvn.wc
> Initializing libsvn._wc
> Initializing libsvn._core
> Calling apr_initialize() from libsvn._core
> Initializing libsvn_swig_py thread key
> Program received signal SIGSEGV, Segmentation fault.
> 0x40314567 in apr_pool_create_ex () from
> /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> #1  0x402fbe3b in svn_swig_py_release_py_lock ()
>     at
> /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> (gdb) up
> #2  0x4021b4fd in _wrap_svn_swig_init_asp_dot_net_hack (self=0x0,
>     args=0x8105844) at subversion/bindings/swig/python/svn_wc.c:23044
> 23044           svn_swig_py_release_py_lock();
> (gdb) up
> #3  0x080cd299 in PyCFunction_Call ()
>
> Here's another crash log:
>>>> import svn.repos
> Initializing libsvn._core
> Calling apr_initialize() from libsvn._core
> Initializing libsvn_swig_py thread key
> (no debugging symbols found)...
> Program received signal SIGSEGV, Segmentation fault.
> 0x40300567 in apr_pool_create_ex () from
> /h/46/james/lib/libsvn_swig_py-1.so.0 (gdb) up
> #1  0x402e7e3b in svn_swig_py_release_py_lock ()
>     at
> /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
> y/swigutil_py.c:78 78          apr_pool_create(&_saved_thread_pool, NULL);
> (gdb) up
> #2  0x40762d34 in _wrap_svn_pool_create (self=0x0, args=0x8105844)
>     at subversion/bindings/swig/python/core.c:7638
> 7638            svn_swig_py_release_py_lock();
> (gdb) up
> #3  0x080cd299 in PyCFunction_Call ()
> (gdb)
>
>
> Here's some test cases:
> NO CRASH: import libsvn.core; import libsvn.wc
> CRASH: import libsvn.wc
> NO CRASH: import libsvn.repos
> CRASH: import libsvn.repos; import libsvn.core; import libsvn.wc
> CRASH: import svn.repos
> NO CRASH: import libsvn.fs
> CRASH: import svn.fs
>
> NOTE:
> If you use the patch I attached earlier in this thread (which adjusts
> the location of the apr_initialize call) then everything works fine.
> I'm not sure why, though -- clearly, from the above output, you can
> see that APR was already initialized by the time we arrived in
> svn_swig_release_py_lock.

How bizarre!
Perhaps running against an APR with debug symbols might bring something 
useful in gdb?

> (Max, what version of APR are you using on your Linux box? I'm using 
> 0.9.6.)

0.9.6 too - I'm using Debian stable.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> David James wrote:
> > On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> >> David James wrote:
> >>>
> >>>   import _wc
> >>>
> >>> Oops! We imported _wc before importing _core. I'm not sure why
> >>> importing _wc before _core leads to crashes on my machine now -- it
> >>> used to work fine.
> >>
> >> Could you try using gdb to figure out where it crashes?
> >>
> >> It does not crash for me, nor would I expect it to, as no use of APR is
> >> made in the _wc initialization function.
> >>
> >> In an "import svn.wc" situation, APR will be initialized when the "import
> >> core" statement in libsvn.wc executes, which follows immediately (except
> >> for some pure python swig internal definitions) after the "import _wc".
>
> > I've figured out the problem: If we're using a threaded APR, and we
> > import libsvn._wc before importing libsvn._core, the
> > svn_swig_release_py_lock function will run before libsvn._core has had
> > a chance to initialize APR.
>
> I was do not have a threaded APR on my primary machine, so I've now compiled
> Subversion on a Linux system which does have threaded APR:
>
> Results:
>
> >>> import svn.wc
> Initializing libsvn._wc
> Initializing libsvn._core
> Initializing libsvn_swig_py threadkey
> >>> # no crash
>
> (messages produced by some strategically placed fprintf(stderr, ...) in the
> C code)
>
> [Tests done with r16474]
>
> Thanks for reverting the earlier workaround.
> Hope you have luck trying to trace the cause of the crash, because it looks
> like a threaded APR is not the cause.

Here's some crash logs from python2.
(I added a few helpful printf() statements.)

Here's an example crash log:

>>> import libsvn.wc
Initializing libsvn._wc
Initializing libsvn._core
Calling apr_initialize() from libsvn._core
Initializing libsvn_swig_py thread key
Program received signal SIGSEGV, Segmentation fault.
0x40314567 in apr_pool_create_ex () from /h/46/james/lib/libsvn_swig_py-1.so.0
(gdb) up
#1  0x402fbe3b in svn_swig_py_release_py_lock ()
    at /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
y/swigutil_py.c:78
78          apr_pool_create(&_saved_thread_pool, NULL);
(gdb) up
#2  0x4021b4fd in _wrap_svn_swig_init_asp_dot_net_hack (self=0x0,
    args=0x8105844) at subversion/bindings/swig/python/svn_wc.c:23044
23044           svn_swig_py_release_py_lock();
(gdb) up
#3  0x080cd299 in PyCFunction_Call ()

Here's another crash log:
>>> import svn.repos
Initializing libsvn._core
Calling apr_initialize() from libsvn._core
Initializing libsvn_swig_py thread key
(no debugging symbols found)...
Program received signal SIGSEGV, Segmentation fault.
0x40300567 in apr_pool_create_ex () from /h/46/james/lib/libsvn_swig_py-1.so.0
(gdb) up
#1  0x402e7e3b in svn_swig_py_release_py_lock ()
    at /nobackup/clgrp/james/trunk/subversion/bindings/swig/python/libsvn_swig_
y/swigutil_py.c:78
78          apr_pool_create(&_saved_thread_pool, NULL);
(gdb) up
#2  0x40762d34 in _wrap_svn_pool_create (self=0x0, args=0x8105844)
    at subversion/bindings/swig/python/core.c:7638
7638            svn_swig_py_release_py_lock();
(gdb) up
#3  0x080cd299 in PyCFunction_Call ()
(gdb)


Here's some test cases:
NO CRASH: import libsvn.core; import libsvn.wc
CRASH: import libsvn.wc
NO CRASH: import libsvn.repos
CRASH: import libsvn.repos; import libsvn.core; import libsvn.wc
CRASH: import svn.repos
NO CRASH: import libsvn.fs
CRASH: import svn.fs

NOTE:
If you use the patch I attached earlier in this thread (which adjusts
the location of the apr_initialize call) then everything works fine.
I'm not sure why, though -- clearly, from the above output, you can
see that APR was already initialized by the time we arrived in
svn_swig_release_py_lock.

(Max, what version of APR are you using on your Linux box? I'm using 0.9.6.)

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
>> David James wrote:
>>>
>>>   import _wc
>>>
>>> Oops! We imported _wc before importing _core. I'm not sure why
>>> importing _wc before _core leads to crashes on my machine now -- it
>>> used to work fine.
>>
>> Could you try using gdb to figure out where it crashes?
>>
>> It does not crash for me, nor would I expect it to, as no use of APR is
>> made in the _wc initialization function.
>>
>> In an "import svn.wc" situation, APR will be initialized when the "import
>> core" statement in libsvn.wc executes, which follows immediately (except
>> for some pure python swig internal definitions) after the "import _wc".

> I've figured out the problem: If we're using a threaded APR, and we
> import libsvn._wc before importing libsvn._core, the
> svn_swig_release_py_lock function will run before libsvn._core has had
> a chance to initialize APR.

I was do not have a threaded APR on my primary machine, so I've now compiled 
Subversion on a Linux system which does have threaded APR:

Results:

>>> import svn.wc
Initializing libsvn._wc
Initializing libsvn._core
Initializing libsvn_swig_py threadkey
>>> # no crash

(messages produced by some strategically placed fprintf(stderr, ...) in the 
C code)

[Tests done with r16474]

Thanks for reverting the earlier workaround.
Hope you have luck trying to trace the cause of the crash, because it looks 
like a threaded APR is not the cause.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> David James wrote:
> > On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> >> David James wrote:
> >>> On 10/4/05, djames@tigris.org <dj...@tigris.org> wrote:
> >>>> Author: djames
> >>>> Date: Tue Oct  4 09:52:26 2005
> >>>> New Revision: 16449
> >>>>
> >>>> Modified:
> >>>>    trunk/build/generator/gen_make.py
> >>>>
> >>>> Log:
> >>>> Ensure that all python modules import libsvn._core before importing
> >>>> any other modules, so that the initialization code is run in the
> >>>> correct
> >>>> order (thus preventing segfaults). We can't implement this trick in
> >>>> SWIG,
> >>>> so we do it by postprocessing the SWIG output files.
> >>>>
> >>>> * build/generator/gen_make.py
> >>>>   (Generator.write): Update generated *.py files to import libsvn._core
> >>>> first.
> ...
> >>> Here's a test case:
> >>>   python -c "import svn.wc"
> >> That fix is _reeeaaaaly_ hacky :-(
> >>
> >> Also, why is it needed at all?
>
> > I hope that this fix is not necessary, or that we can find a different
> > way of implementing it. :)
> >
> >> The testcase completes successfully for me without this revision, plus, I
> >> manually examined the generated .py files and found that _core was being
> >> imported soon enough anyway without this fix.
>
> > Is it? On my machine, libsvn/wc.py starts like this:
> >   # This file was created automatically by SWIG.
> >   # Don't modify this file, modify the SWIG interface instead.
> >   # This file is compatible with both classic and new-style classes.
> >
> >   import _wc
> >
> > Oops! We imported _wc before importing _core. I'm not sure why
> > importing _wc before _core leads to crashes on my machine now -- it
> > used to work fine.
>
> Could you try using gdb to figure out where it crashes?
>
> It does not crash for me, nor would I expect it to, as no use of APR is made
> in the _wc initialization function.
>
> In an "import svn.wc" situation, APR will be initialized when the "import
> core" statement in libsvn.wc executes, which follows immediately (except for
> some pure python swig internal definitions) after the "import _wc".
I've figured out the problem: If we're using a threaded APR, and we
import libsvn._wc before importing libsvn._core, the
svn_swig_release_py_lock function will run before libsvn._core has had
a chance to initialize APR. Does this fix look reasonable?

[[[

Move apr_initialize calls into swigutil_py.c so that APR will be initialized
when svn_swig_py_release_lock is called.

* swig/core.i: Remove calls to apr_initialize and apr_terminate.
* swig/python/libsvn_swig_py/swigutil_py.c
  (svn_swig_py_release_lock): Initialize APR, if it has not been initialized
  yet. Initialize atexit(apr_terminate) exit handler.

]]]

Cheers,

David
--
David James -- http://www.cs.toronto.edu/~james

Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
>> David James wrote:
>>> On 10/4/05, djames@tigris.org <dj...@tigris.org> wrote:
>>>> Author: djames
>>>> Date: Tue Oct  4 09:52:26 2005
>>>> New Revision: 16449
>>>>
>>>> Modified:
>>>>    trunk/build/generator/gen_make.py
>>>>
>>>> Log:
>>>> Ensure that all python modules import libsvn._core before importing
>>>> any other modules, so that the initialization code is run in the 
>>>> correct
>>>> order (thus preventing segfaults). We can't implement this trick in 
>>>> SWIG,
>>>> so we do it by postprocessing the SWIG output files.
>>>>
>>>> * build/generator/gen_make.py
>>>>   (Generator.write): Update generated *.py files to import libsvn._core
>>>> first.
...
>>> Here's a test case:
>>>   python -c "import svn.wc"
>> That fix is _reeeaaaaly_ hacky :-(
>>
>> Also, why is it needed at all?

> I hope that this fix is not necessary, or that we can find a different
> way of implementing it. :)
>
>> The testcase completes successfully for me without this revision, plus, I
>> manually examined the generated .py files and found that _core was being
>> imported soon enough anyway without this fix.

> Is it? On my machine, libsvn/wc.py starts like this:
>   # This file was created automatically by SWIG.
>   # Don't modify this file, modify the SWIG interface instead.
>   # This file is compatible with both classic and new-style classes.
>
>   import _wc
>
> Oops! We imported _wc before importing _core. I'm not sure why
> importing _wc before _core leads to crashes on my machine now -- it
> used to work fine.

Could you try using gdb to figure out where it crashes?

It does not crash for me, nor would I expect it to, as no use of APR is made 
in the _wc initialization function.

In an "import svn.wc" situation, APR will be initialized when the "import 
core" statement in libsvn.wc executes, which follows immediately (except for 
some pure python swig internal definitions) after the "import _wc".

Max.




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r16449 - trunk/build/generator

Posted by David James <ja...@gmail.com>.
On 10/4/05, Max Bowsher <ma...@ukf.net> wrote:
> David James wrote:
> > On 10/4/05, djames@tigris.org <dj...@tigris.org> wrote:
> >> Author: djames
> >> Date: Tue Oct  4 09:52:26 2005
> >> New Revision: 16449
> >>
> >> Modified:
> >>    trunk/build/generator/gen_make.py
> >>
> >> Log:
> >> Ensure that all python modules import libsvn._core before importing
> >> any other modules, so that the initialization code is run in the correct
> >> order (thus preventing segfaults). We can't implement this trick in SWIG,
> >> so we do it by postprocessing the SWIG output files.
> >>
> >> * build/generator/gen_make.py
> >>   (Generator.write): Update generated *.py files to import libsvn._core
> >> first.
> >>
> >>
> >>
> >> Modified: trunk/build/generator/gen_make.py
> >> Url:
> >> http://svn.collab.net/viewcvs/svn/trunk/build/generator/gen_make.py?rev=16449&p1=trunk/build/generator/gen_make.py&p2=trunk/build/generator/gen_make.py&r1=16448&r2=16449
> >> ==============================================================================
> >> --- trunk/build/generator/gen_make.py   (original) +++
> >> trunk/build/generator/gen_make.py   Tue Oct  4 09:52:26 2005 @@ -1,10
> >> +1,10 @@ -#
> >>  # gen_make.py -- generate makefiles and dependencies
> >>  #
> >>
> >>  import os
> >>  import sys
> >>  import string
> >> +import re
> >>
> >>  import gen_base
> >>  import generator.util.executable
> >> @@ -209,7 +209,20 @@
> >>          'cp -pf $(abs_srcdir)/%s/*.i ' % source_dir +
> >>          '$(abs_builddir)/%s; fi\n' % source_dir +
> >>          '\t$(SWIG) $(SWIG_INCLUDES) %s ' % opts +
> >> -        '-o $@ $(abs_builddir)/%s\n' % source +
> >> +        '-o $@ $(abs_builddir)/%s\n' % source
> >> +      )
> >> +      if objname.lang == "python":
> >> +        # Ensure that all python modules import libsvn._core before
> >> importing +        # any other modules, so that the initialization code
> >> is
> >> run in the +        # correct order. We can't implement this trick in
> >> SWIG, so we do it +        # by postprocessing the SWIG output files.
> >> +        pyfile = re.sub(r"(?:svn_)?(\w+)\.c$",r"\1.py", str(objname))
> >> +        self.ofile.write(
> >> +          '\tmv %s %s-swig ' % (pyfile, pyfile) +
> >> +          '&& (echo import _core | cat - %s-swig > %s) ' % (pyfile,
> >> pyfile) + +          '&& rm %s-swig\n' % (pyfile)
> >> +        )
> >> +      self.ofile.write(
> >>          'autogen-swig-%s: copy-swig-%s\n' % (short[objname.lang],
> >>          objname) + 'copy-swig-%s: %s\n' % (objname, objname) +
> >>          '\t@if test $(abs_srcdir) != $(abs_builddir) -a ' +
> >
> > Any suggestions for how we can port this fix to Windows?
> >
> > Here's a test case:
> >   python -c "import svn.wc"
> That fix is _reeeaaaaly_ hacky :-(
>
> Also, why is it needed at all?
I hope that this fix is not necessary, or that we can find a different
way of implementing it. :)

> The testcase completes successfully for me without this revision, plus, I
> manually examined the generated .py files and found that _core was being
> imported soon enough anyway without this fix.
Is it? On my machine, libsvn/wc.py starts like this:
  # This file was created automatically by SWIG.
  # Don't modify this file, modify the SWIG interface instead.
  # This file is compatible with both classic and new-style classes.

  import _wc

Oops! We imported _wc before importing _core. I'm not sure why
importing _wc before _core leads to crashes on my machine now -- it
used to work fine.

Cheers,

David


--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r16449 - trunk/build/generator

Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> On 10/4/05, djames@tigris.org <dj...@tigris.org> wrote:
>> Author: djames
>> Date: Tue Oct  4 09:52:26 2005
>> New Revision: 16449
>>
>> Modified:
>>    trunk/build/generator/gen_make.py
>>
>> Log:
>> Ensure that all python modules import libsvn._core before importing
>> any other modules, so that the initialization code is run in the correct
>> order (thus preventing segfaults). We can't implement this trick in SWIG,
>> so we do it by postprocessing the SWIG output files.
>>
>> * build/generator/gen_make.py
>>   (Generator.write): Update generated *.py files to import libsvn._core
>> first.
>>
>>
>>
>> Modified: trunk/build/generator/gen_make.py
>> Url:
>> http://svn.collab.net/viewcvs/svn/trunk/build/generator/gen_make.py?rev=16449&p1=trunk/build/generator/gen_make.py&p2=trunk/build/generator/gen_make.py&r1=16448&r2=16449
>> ==============================================================================
>> --- trunk/build/generator/gen_make.py   (original) +++
>> trunk/build/generator/gen_make.py   Tue Oct  4 09:52:26 2005 @@ -1,10
>> +1,10 @@ -#
>>  # gen_make.py -- generate makefiles and dependencies
>>  #
>>
>>  import os
>>  import sys
>>  import string
>> +import re
>>
>>  import gen_base
>>  import generator.util.executable
>> @@ -209,7 +209,20 @@
>>          'cp -pf $(abs_srcdir)/%s/*.i ' % source_dir +
>>          '$(abs_builddir)/%s; fi\n' % source_dir +
>>          '\t$(SWIG) $(SWIG_INCLUDES) %s ' % opts +
>> -        '-o $@ $(abs_builddir)/%s\n' % source +
>> +        '-o $@ $(abs_builddir)/%s\n' % source
>> +      )
>> +      if objname.lang == "python":
>> +        # Ensure that all python modules import libsvn._core before
>> importing +        # any other modules, so that the initialization code 
>> is
>> run in the +        # correct order. We can't implement this trick in
>> SWIG, so we do it +        # by postprocessing the SWIG output files.
>> +        pyfile = re.sub(r"(?:svn_)?(\w+)\.c$",r"\1.py", str(objname))
>> +        self.ofile.write(
>> +          '\tmv %s %s-swig ' % (pyfile, pyfile) +
>> +          '&& (echo import _core | cat - %s-swig > %s) ' % (pyfile,
>> pyfile) + +          '&& rm %s-swig\n' % (pyfile)
>> +        )
>> +      self.ofile.write(
>>          'autogen-swig-%s: copy-swig-%s\n' % (short[objname.lang],
>>          objname) + 'copy-swig-%s: %s\n' % (objname, objname) +
>>          '\t@if test $(abs_srcdir) != $(abs_builddir) -a ' +
>
> Any suggestions for how we can port this fix to Windows?
>
> Here's a test case:
>   python -c "import svn.wc"

That fix is _reeeaaaaly_ hacky :-(

Also, why is it needed at all?

The testcase completes successfully for me without this revision, plus, I 
manually examined the generated .py files and found that _core was being 
imported soon enough anyway without this fix.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org