classification
Title: Remove old ctypes callback workaround: creating the first instance of a callback
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ideasman42, ned.deily, vstinner
Priority: normal Keywords: patch

Created on 2018-12-18 12:04 by vstinner, last changed 2021-01-25 19:31 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11211 merged vstinner, 2018-12-18 12:53
Messages (7)
msg332054 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-12-18 12:04
ctypes._reset_cache() contains the following code:

    # XXX for whatever reasons, creating the first instance of a callback
    # function is needed for the unittests on Win64 to succeed.  This MAY
    # be a compiler bug, since the problem occurs only when _ctypes is
    # compiled with the MS SDK compiler.  Or an uninitialized variable?
    CFUNCTYPE(c_int)(lambda: None)

This code has been added 11 years ago:

commit 674e9389e9fdadd622829f4833367ac3b38475b5
Author: Thomas Heller <theller@ctypes.org>
Date:   Fri Aug 31 13:06:44 2007 +0000

    Add a workaround for a strange bug on win64, when _ctypes is compiled
    with the SDK compiler.  This should fix the failing
    Lib\ctypes\test\test_as_parameter.py test.

diff --git a/Lib/ctypes/__init__.py b/Lib/ctypes/__init__.py
index cdf6d36e47..f55d194b8f 100644
--- a/Lib/ctypes/__init__.py
+++ b/Lib/ctypes/__init__.py
@@ -535,3 +535,9 @@ for kind in [c_ushort, c_uint, c_ulong, c_ulonglong]:
     elif sizeof(kind) == 4: c_uint32 = kind
     elif sizeof(kind) == 8: c_uint64 = kind
 del(kind)
+
+# XXX for whatever reasons, creating the first instance of a callback
+# function is needed for the unittests on Win64 to succeed.  This MAY
+# be a compiler bug, since the problem occurs only when _ctypes is
+# compiled with the MS SDK compiler.  Or an uninitialized variable?
+CFUNCTYPE(c_int)(lambda: None)

--

This call is removed from Fedora package by the following patch:

https://src.fedoraproject.org/rpms/python3/blob/master/f/00155-avoid-ctypes-thunks.patch

Extract of Fedora python3.spec:

# 00155 #
# Avoid allocating thunks in ctypes unless absolutely necessary, to avoid
# generating SELinux denials on "import ctypes" and "import uuid" when
# embedding Python within httpd
# See https://bugzilla.redhat.com/show_bug.cgi?id=814391
Patch155: 00155-avoid-ctypes-thunks.patch

The patch has been added 6 years ago in Fedora:

commit 8a28107df1670a03a12cf6a7787160f103d8d8c8
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Apr 20 15:28:39 2012 -0400

    3.2.3-4: avoid allocating thunks in ctypes unless absolutely necessary (patch 155; rhbz#814391)
    
    * Fri Apr 20 2012 David Malcolm <dmalcolm@redhat.com> - 3.2.3-4
    - avoid allocating thunks in ctypes unless absolutely necessary, to avoid
    generating SELinux denials on "import ctypes" and "import uuid" when embedding
    Python within httpd (patch 155; rhbz#814391)

https://src.fedoraproject.org/rpms/python3/c/8a28107df1670a03a12cf6a7787160f103d8d8c8?branch=master

--

I don't understand the purpose of the workaround and ctypes is working well on Fedora. I propose to also remove the workaround in the master branch.

In case of doubt, I prefer to keep the workaround in Python 3.7.

Attached PR removes the workaround.
msg332059 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-12-18 13:47
New changeset e6b247c8e524dbe5fc03b3492f628d0d5348bc49 by Victor Stinner in branch 'master':
bpo-35523: Remove ctypes callback workaround (GH-11211)
https://github.com/python/cpython/commit/e6b247c8e524dbe5fc03b3492f628d0d5348bc49
msg332061 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-12-18 14:10
I will keep an eye on buildbots next days.
msg385596 - (view) Author: Campbell Barton (ideasman42) * Date: 2021-01-25 01:49
Could this fix be applied to Python 3.7x?

Currently the visual effects platform for 2021 is using Python 3.7.x, see: https://vfxplatform.com

Which means anyone using the VFX platform can run into this bug.
msg385597 - (view) Author: Campbell Barton (ideasman42) * Date: 2021-01-25 01:51
Note, this bug is something we ran into in Blender, see: https://developer.blender.org/T84752
msg385613 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-01-25 10:20
Ned Deily (Python 3.7 release manager): do you want an exception to backport a bugfix?

Campbell: Python 3.7 no longer accept bug fixes, only security fixes.
https://devguide.python.org/#status-of-python-branches

As I wrote previously, I'm not comfortable to make such change in a 3.7.x minor release, even if we didn't get any bug report since this change was done in Python 3.8.
msg385650 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-01-25 19:31
It is certainly not good that some of the vfx users are seeing a crash. On the other hand, at this stage of Python 3.7's life cycle only security-related fixes are accepted as a primary goal is to keep 3.7 as stable and unchanged as possible for those users still using it. We always recommend using a fully supported version, today either 3.8.7 (for several more months) or 3.9.1. It is unfortunate that the Fedora patch wasn't applied upstream earlier when it would have been exposed across all platforms. If we could be more certain that applying it now would have no side effects on other platforms, it might be an easier call. As long as the vfx project is building and supporting its own copies of Python 3.7, I think it would be better for them to carry the Fedora patch locally if they want it until they migrate to a newer version of Python.
History
Date User Action Args
2021-01-25 19:31:11ned.deilysetmessages: + msg385650
2021-01-25 10:20:00vstinnersetnosy: + ned.deily
messages: + msg385613
2021-01-25 01:51:01ideasman42setmessages: + msg385597
2021-01-25 01:49:21ideasman42setnosy: + ideasman42
messages: + msg385596
2018-12-18 14:10:26vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg332061

stage: patch review -> resolved
2018-12-18 13:47:29vstinnersetmessages: + msg332059
2018-12-18 12:57:10vstinnersettitle: Remove ctypes old workaround: creating the first instance of a callback -> Remove old ctypes callback workaround: creating the first instance of a callback
2018-12-18 12:53:11vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request10448
2018-12-18 12:04:01vstinnercreate