Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows

On 2020/05/22 8:02, Yasuhito FUTATSUKI wrote:
> On 2020/05/20 21:54, Jun Omae wrote:
>> On Mon, May 18, 2020 at 8:25 AM Yasuhito FUTATSUKI <futatuki_at_poem.co.jp> wrote:

>>> Jun and I found it is caused by long existing bug on SWIG Python bindings,
>>> since before swig-py3 branch was merged (thus this bug exists on 1.13.x
>>> and 1.10.3 branches). On SWIG Python bindings, APIs which have multiple
>>> apr_pool_t * argments cannot be wrapped correctly.
>>>
>>> If we call such a wrapper function with specifying multiple pool argments
>>> explicitly, only last one is used for *all* apr_pool_t * arguments when
>>> it calls C API.
>>
>> Created global-py-pool-ref-count.diff patch to solve the multiple apr_pool *
>> pointers issue in Python bindings.
>>
>> Verified no assertions while running check-swig-py with the following
>> environments:
>>
>> - (Python 3.5.2, 3.6.10, 3.7.7, 3.8.3, 3.9.0a6) x (SWIG 4.0.1)
>> - (Python 2.7.12) x (SWIG 1.3.40, 2.0.12, 3.0.12)
>
> Thank you very much!
>
> With this patch, I confirmed that each apr_pool * argment in C API supplied
> from corresponding pool object if it is supplied by the Python wrapper function
> (by reading _wrap_svn_client_propget5() in svn_client.c generated by
> SWIG 3.0.12, for Python 2).
>
> However, I couldn't confirm that the "_parent_pool" attribute in wrapper
> object returned by wrapper function point to "result_pool" object in such case,
> yet. I doubt it still points to "scratch_pool" object because variable
> "_global_py_pool" points to "scratch_pool" object when both of "result_pool"
> and "scratch_pool" are provided on such APIs.

In "out" and "argout" typemaps, "_parent_pool" attribute of wrapper objects
are set from "_global_py_pool".

With this additional hunks for SubversionClientTestCase.test_conflict(),
[[[
Index: subversion/bindings/swig/python/tests/client.py
===================================================================
--- subversion/bindings/swig/python/tests/client.py (revision 1877963)
+++ subversion/bindings/swig/python/tests/client.py (working copy)
@@ -592,10 +603,14 @@
     client.update4((path,), rev, core.svn_depth_unknown, True, False, False,
                    False, False, self.client_ctx)
 
- pool = core.Pool()
- conflict = client.conflict_get(trunk_path, self.client_ctx, pool)
+ result_pool = core.Pool()
+ scratch_pool = core.Pool()
+ conflict = client.conflict_get(trunk_path, self.client_ctx,
+ result_pool, scratch_pool)
 
     self.assertTrue(isinstance(conflict, client.svn_client_conflict_t))
+ self.assertNotEqual(conflict._parent_pool, scratch_pool)
+ self.assertEqual(conflict._parent_pool, result_pool)
 
     conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx)
 
@@ -602,7 +617,8 @@
     self.assertTrue(isinstance(conflict_opts, list))
     self.assert_all_instances_of(conflict_opts, client.svn_client_conflict_option_t)
 
- pool.clear()
+ scratch_pool.clear()
+ result_pool.clear()
 
   @unittest.skip("experimental API, not currently exposed")
   def test_shelf(self):
]]]
this test fails.

[[[
======================================================================
FAIL: test_conflict (client.SubversionClientTestCase)
Test conflict api.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/futatuki/work/subversion/vwc/trunk/subversion/bindings/swig/python/tests/client.py", line 612, in test_conflict
    self.assertNotEqual(conflict._parent_pool, scratch_pool)
AssertionError: <libsvn.core.apr_pool_t; proxy of <Swig Object of type 'apr_pool_t *' at 0x80acf5180> > == <libsvn.core.apr_pool_t; proxy of <Swig Object of type 'apr_pool_t *' at 0x80acf5180> >

----------------------------------------------------------------------
]]]

I think it can be fixed by overriding _global_py_pool to $input in "in"
typemap, when $1 is updated. It assumes that there are no APIs that
uses two or more result_pool like apr_pool_t * argument for allocationg
return value.

I tweaked Jun's patch to do it.
(Attached file: global-py-pool-ref-count2-diff.txt)

Tested on FreeBSD 11, with Python 2.7(debug), Python 3.7(non debug)

Cheers,

-- 
Yasuhito FUTATSUKI <futatuki_at_poem.co.jp>/<futatuki_at_yf.bsdclub.org>