Merge pull request #261 from python-greenlet/issue252

Fix several greenlet leaks

.github/workflows/tests.yml

@@ -8,6 +8,7 @@ env:
   PYTHONUNBUFFERED: 1
   PYTHONDONTWRITEBYTECODE: 1
   PYTHONDEVMODE: 1
+  PYTHONFAULTHANDLER: 1
   PIP_UPGRADE_STRATEGY: eager
   # Don't get warnings about Python 2 support being deprecated. We
   # know. The env var works for pip 20.
@@ -24,7 +25,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        python-version: [2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10.0-rc.1]
+        python-version: [2.7, 3.5, 3.6, 3.7, 3.8, 3.9, "3.10"]
         os: [ubuntu-latest, macos-latest]
     steps:
     - uses: actions/checkout@v2
@@ -43,10 +44,18 @@ jobs:
       run: |
         python -m pip install -U pip setuptools wheel
         python -m pip install -U twine
+        python -m pip install -q -U 'faulthandler; python_version == "2.7" and platform_python_implementation == "CPython"'
     - name: Install greenlet
       run: |
         python setup.py bdist_wheel
         python -m pip install -U -e ".[test,docs]"
+      env:
+        # Ensure we test with assertions enabled.
+        # As opposed to the manylinux builds, which we distribute and
+        # thus only use O3 (because Ofast enables fast-math, which has
+        # process-wide effects), we test with Ofast here, because we
+        # expect that some people will compile it themselves with that setting.
+        CPPFLAGS: "-Ofast -UNDEBUG"
     - name: Check greenlet build
       run: |
         ls -l dist
@@ -58,11 +67,10 @@ jobs:
         path: dist/*whl
     - name: Test
       run: |
+        python -c 'import faulthandler; assert faulthandler.is_enabled()'
+        python -c 'import greenlet._greenlet as G; assert G.GREENLET_USE_STANDARD_THREADING'
         python -m unittest discover -v greenlet.tests
     - name: Doctest
-      # FIXME: This conditional can go away when a Sphinx greater than 4.1.2
-      # is released. See https://github.com/sphinx-doc/sphinx/issues/9512
-      if: matrix.python-version != '3.10.0-rc.1'
       run: |
         sphinx-build -b doctest -d docs/_build/doctrees2 docs docs/_build/doctest2
     - name: Publish package to PyPI (mac)
@@ -75,6 +83,41 @@ jobs:
       run: |
         twine upload --skip-existing dist/*
 
+  test_non_standard_thread:
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        python-version: [2.7, 3.5, "3.10"]
+        os: [ubuntu-latest, macos-latest]
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Pip cache
+      uses: actions/cache@v2
+      with:
+        path: ~/.cache/pip
+        key: ${{ runner.os }}-pip-ns-${{ hashFiles('setup.*') }}
+        restore-keys: |
+          ${{ runner.os }}-pip-ns-
+    - name: Install dependencies
+      run: |
+        python -m pip install -U pip setuptools wheel
+        python -m pip install -U twine
+        python -m pip install -q -U 'faulthandler; python_version == "2.7" and platform_python_implementation == "CPython"'
+    - name: Install greenlet
+      env:
+        CPPFLAGS: "-DG_USE_STANDARD_THREADING=0 -UNDEBUG -Ofast"
+      run: |
+        python setup.py bdist_wheel
+        python -m pip install -U -v -e ".[test,docs]"
+    - name: Test
+      run: |
+        python -c 'import faulthandler; assert faulthandler.is_enabled()'
+        python -c 'import greenlet._greenlet as G; assert not G.GREENLET_USE_STANDARD_THREADING'
+        python -m unittest discover -v greenlet.tests
 
   manylinux:
 

.gitignore

@@ -11,3 +11,4 @@ greenlet.egg-info/
 __pycache__/
 /.ropeproject/
 /MANIFEST
+benchmarks/*.json

.pylintrc

@@ -0,0 +1,130 @@
+[MASTER]
+extension-pkg-whitelist=gevent.greenlet,gevent.libuv._corecffi,gevent.libev._corecffi,gevent.libev._corecffi.lib,gevent.local,gevent._ident
+
+# Control the amount of potential inferred values when inferring a single
+# object. This can help the performance when dealing with large functions or
+# complex, nested conditions.
+# gevent: The changes for Python 3.7 in _ssl3.py lead to infinite recursion
+# in pylint 2.3.1/astroid 2.2.5 in that file unless we this this to 1
+# from the default of 100.
+limit-inference-results=1
+
+[MESSAGES CONTROL]
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifier separated by comma (,) or put this option
+# multiple time (only on the command line, not in the configuration file where
+# it should appear only once).
+# NOTE: comments must go ABOVE the statement. In Python 2, mixing in
+# comments disables all directives that follow, while in Python 3, putting
+# comments at the end of the line does the same thing (though Py3 supports
+# mixing)
+
+
+# invalid-name, ; We get lots of these, especially in scripts. should fix many of them
+# protected-access, ; We have many cases of this; legit ones need to be examinid and commented, then this removed
+# no-self-use, ; common in superclasses with extension points
+# too-few-public-methods, ; Exception and marker classes get tagged with this
+# exec-used, ; should tag individual instances with this, there are some but not too many
+# global-statement,     ; should tag individual instances
+# multiple-statements, ; "from gevent import monkey; monkey.patch_all()"
+# locally-disabled, ; yes, we know we're doing this. don't replace one warning with another
+# cyclic-import, ; most of these are deferred imports
+# too-many-arguments, ; these are almost always because that's what the stdlib does
+# redefined-builtin, ; likewise: these tend to be keyword arguments like len= in the stdlib
+# undefined-all-variable,     ; XXX: This crashes with pylint 1.5.4 on Travis (but not locally on Py2/3
+#     ; or landscape.io on Py3). The file causing the problem is unclear. UPDATE: identified and disabled
+#   that file.
+#   see https://github.com/PyCQA/pylint/issues/846
+# useless-suppression: the only way to avoid repeating it for specific statements everywhere that we
+#   do Py2/Py3 stuff is to put it here. Sadly this means that we might get better but not realize it.
+# duplicate-code: Yeah, the compatibility ssl modules are much the same
+# In pylint 1.8.0, inconsistent-return-statements are created for the wrong reasons.
+# This code raises it, even though there's only one return (the implicit 'return None' is presumably
+# what triggers it):
+#  def foo():
+#    if baz:
+#      return 1
+# In Pylint 2dev1, needed for Python 3.7, we get spurious 'useless return' errors:
+# @property
+# def foo(self):
+#   return None # generates useless-return
+# Pylint 2.4 adds import-outside-toplevel. But we do that a lot to defer imports because of patching.
+# Pylint 2.4 adds self-assigning-variable. But we do *that* to avoid unused-import when we
+# "export" the variable and don't have a __all__.
+# Pylint 2.6+ adds some python-3-only things that don't apply: raise-missing-from, super-with-arguments, consider-using-f-string, redundant-u-string-prefix
+disable=missing-docstring,
+    ungrouped-imports,
+    invalid-name,
+    protected-access,
+    no-self-use,
+    too-few-public-methods,
+    exec-used,
+    global-statement,
+    multiple-statements,
+    locally-disabled,
+    cyclic-import,
+    too-many-arguments,
+    redefined-builtin,
+    useless-suppression,
+    duplicate-code,
+    undefined-all-variable,
+    inconsistent-return-statements,
+    useless-return,
+    useless-object-inheritance,
+    import-outside-toplevel,
+    self-assigning-variable,
+    raise-missing-from,
+    super-with-arguments,
+    consider-using-f-string,
+    redundant-u-string-prefix
+
+[FORMAT]
+# duplicated from setup.cfg
+max-line-length=160
+max-module-lines=1100
+
+[MISCELLANEOUS]
+# List of note tags to take in consideration, separated by a comma.
+#notes=FIXME,XXX,TODO
+# Disable that, we don't want them in the report (???)
+notes=
+
+[VARIABLES]
+
+dummy-variables-rgx=_.*
+
+[TYPECHECK]
+
+# List of members which are set dynamically and missed by pylint inference
+# system, and so shouldn't trigger E1101 when accessed. Python regular
+# expressions are accepted.
+# gevent: this is helpful for py3/py2 code.
+generated-members=exc_clear
+
+# List of classes names for which member attributes should not be checked
+# (useful for classes with attributes dynamically set). This can work
+# with qualified names.
+#ignored-classes=SSLContext, SSLSocket, greenlet, Greenlet, parent, dead
+
+
+# List of module names for which member attributes should not be checked
+# (useful for modules/projects where namespaces are manipulated during runtime
+# and thus existing member attributes cannot be deduced by static analysis. It
+# supports qualified module names, as well as Unix pattern matching.
+ignored-modules=gevent._corecffi,gevent.os,os,threading,gevent.libev.corecffi,gevent.socket,gevent.core,gevent.testing.support
+
+[DESIGN]
+max-attributes=12
+max-parents=10
+
+[BASIC]
+bad-functions=input
+# Prospector turns ot unsafe-load-any-extension by default, but
+# pylint leaves it off. This is the proximal cause of the
+# undefined-all-variable crash.
+unsafe-load-any-extension = yes
+
+# Local Variables:
+# mode: conf
+# End:

CHANGES.rst

@@ -2,11 +2,20 @@
  Changes
 =========
 
-1.2.0 (unreleased)
+2.0.0 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Drop support for very old versions of GCC and MSVC.
 
+- Compilation now requires a compiler that either supports C++11 or
+  has some other intrinsic way to create thread local variables; for
+  older GCC, clang and SunStudio we use ``__thread``, while for older
+  MSVC we use ``__declspec(thread)``.
+
+- Fix several leaks that could occur when using greenlets from
+  multiple threads. For example, it is no longer necessary to call
+  ``getcurrent()`` before exiting a thread to allow its main greenlet
+  to be cleaned up. See `issue 252 <https://github.com/python-greenlet/greenlet/issues/251>`_.
 
 1.1.2 (2021-09-29)
 ==================
@@ -229,9 +238,9 @@ Packaging Changes
 =====
 * Greenlet has an instance dictionary now, which means it can be
   used for implementing greenlet local storage, etc. However, this
-  might introduce incompatibility if subclasses have __dict__ in their
-  __slots__. Classes like that will fail, because greenlet already
-  has __dict__ out of the box.
+  might introduce incompatibility if subclasses have ``__dict__`` in their
+  ``__slots__``. Classes like that will fail, because greenlet already
+  has ``__dict__`` out of the box.
 * Greenlet no longer leaks memory after thread termination, as long as
   terminated thread has no running greenlets left at the time.
 * Add support for debian sparc and openbsd5-sparc64

MANIFEST.in

@@ -4,6 +4,7 @@
 recursive-include src *.py
 recursive-include src *.c
 recursive-include src *.cpp
+recursive-include src *.hpp
 recursive-include src *.h
 recursive-include src *.cmd
 recursive-include src *.asm
@@ -28,6 +29,7 @@ include *.cfg
 include *.py
 include *.ini
 include .clang-format
+include .pylintrc
 
 recursive-include appveyor *.cmd
 recursive-include appveyor *.ps1

appveyor.yml

@@ -20,6 +20,8 @@ environment:
     # Use a fixed hash seed for reproducability
     PYTHONHASHSEED: 8675309
     PYTHONDEVMODE: 1
+    PYTHONFAULTHANDLER: 1
+    PYTHONUNBUFFERED: 1
     # Don't get warnings about Python 2 support being deprecated. We
     # know.
     PIP_NO_PYTHON_VERSION_WARNING: 1
@@ -35,7 +37,7 @@ environment:
   matrix:
     # http://www.appveyor.com/docs/installed-software#python
     - PYTHON: "C:\\Python310-x64"
-      PYTHON_VERSION: "3.10.0rc2"
+      PYTHON_VERSION: "3.10.0"
       PYTHON_ARCH: "64"
       PYTHON_EXE: python
       APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
@@ -118,8 +120,8 @@ install:
         Where-Object pullRequestId -eq $env:APPVEYOR_PULL_REQUEST_NUMBER)[0].buildNumber) { `
           throw "There are newer queued builds for this pull request, failing early." }
   ## Debugging
-  # - ECHO "Filesystem root:"
-  # - ps: "ls \"C:/\""
+  - ECHO "Filesystem root:"
+  - ps: "ls \"C:/\""
 
   # - ECHO "Installed SDKs:"
   # - ps: "if(Test-Path(\"C:/Program Files/Microsoft SDKs/Windows\")) {ls \"C:/Program Files/Microsoft SDKs/Windows\";}"