Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several greenlet leaks #261

Merged
merged 65 commits into from
Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
54105f0
A bare Event.wait() with no timeout is an invitation to hang the proc…
jamadden Sep 29, 2021
24e4cba
See if all compilers can work in C++ mode.
jamadden Sep 28, 2021
e960906
Debugging: Do thread_local objects work?
jamadden Sep 28, 2021
1921580
Drop Python 2.7 on Windows; the compiler is far too old.
jamadden Sep 28, 2021
8dcf174
Try declspec(thread) for old MSC.
jamadden Sep 29, 2021
82be175
Port all global variables to a threadlocal struct.
jamadden Sep 29, 2021
ff3c7c2
Add benchmarks.
jamadden Sep 29, 2021
024d3d7
debug.
jamadden Sep 29, 2021
1b7906c
more debugging.
jamadden Sep 29, 2021
7e24943
g_switchstack was using a stack local variable, which it can't do.
jamadden Sep 29, 2021
be25e20
Perhaps MSVC was combining the two apparent copies of g_thread_state_…
jamadden Sep 29, 2021
2cd7d74
Try a separate function.
jamadden Sep 29, 2021
c79a40a
Remove debugging and update comments.
jamadden Sep 29, 2021
d59bb5b
Checkpoint on #252.
jamadden Sep 30, 2021
2f22881
Fix compilation on MSVC.
jamadden Sep 30, 2021
caf72df
Enable debug prints, windows is crashing.
jamadden Oct 1, 2021
32e7124
You can't call PyObject_Print if there's an error set.
jamadden Oct 1, 2021
cb14308
More debugging
jamadden Oct 1, 2021
0b19e32
Hmm, sometimes the current greenlet was null on Windows? How?
jamadden Oct 1, 2021
9ad0d55
Checkpoint moving to a more flexible use of TLS.
jamadden Oct 1, 2021
340664f
The indirect TLS support appears to be working, and I have fixed #252
jamadden Oct 2, 2021
5e5fa2a
Some debugging.
jamadden Oct 4, 2021
3a2a33d
Sometimes we lose a greenlet.
jamadden Oct 4, 2021
da7f417
Fix the leak on older versions of CPython.
jamadden Oct 4, 2021
cf4f729
Some platforms don't leak the greenlet when explicitly referenced.
jamadden Oct 4, 2021
a421362
Queue pending cleanups to reduce the number of Py_AddPendingCall slot…
jamadden Oct 4, 2021
f80d0fb
Separating out concerns.
jamadden Oct 5, 2021
3211747
Progress on supporting older compilers.
jamadden Oct 5, 2021
a22f68e
Thus begins the fight against MSVC 9.
jamadden Oct 5, 2021
62fac36
Use the old idiom.
jamadden Oct 5, 2021
3662fba
Stop including headers we know aren't present. We got to that point o…
jamadden Oct 5, 2021
9256f4c
Trying MFC classes for the lock. Not sure if that will work.
jamadden Oct 5, 2021
6c350ef
MFC was a no-go. So use the win32 API.
jamadden Oct 5, 2021
4a35c59
Introduce PyMainGreenlet as a subtype of greenlet.
jamadden Oct 5, 2021
e25aea0
Fix bad descriptor error on Python 2.7 for the main greenlet, and com…
jamadden Oct 6, 2021
5bf7906
Implement cleanup using the thread dictionary on platforms that requi…
jamadden Oct 6, 2021
9b94af6
Add GHA test cases that disable the use of standard threading.
jamadden Oct 6, 2021
fadae21
Allow using the PyThread_ APIs back on Python 2.7 as well. Add tox.in…
jamadden Oct 6, 2021
bcaf0f1
Test 2.7 non-standard threading on GHA. Use 3.10 final on Appveyor.
jamadden Oct 6, 2021
a7dbfe4
Eliminate a few unneeded calls te GET_THREAD_STATE.
jamadden Oct 6, 2021
c7841e3
Passing the thread state through all the calls was too much, see if w…
jamadden Oct 6, 2021
04c9166
Try another way to pass the info around.
jamadden Oct 6, 2021
6061284
Reducing preprocessor reliance.
jamadden Oct 7, 2021
99d7f0d
Simplify thread state cleanup.
jamadden Oct 7, 2021
af9b38c
Fix a faulty assertion.
jamadden Oct 7, 2021
0792917
Tweak compiler flags.
jamadden Oct 7, 2021
5aee563
More Py2/Py3 cleanups.
jamadden Oct 7, 2021
7f31618
Py2.7 testing.
jamadden Oct 7, 2021
4ec6c35
Remove more debugging code.
jamadden Oct 7, 2021
d18ac12
Update documentation.
jamadden Oct 7, 2021
0da2574
Ensure more tests don't leak greenlets.
jamadden Oct 7, 2021
b8a8a84
Try to clean up some more leaked greenlets seen on linux.
jamadden Oct 7, 2021
6358768
Debugging
jamadden Oct 7, 2021
1a28424
More debugging
jamadden Oct 7, 2021
4099794
Yet more debugging.
jamadden Oct 7, 2021
3d155fa
Yet more debugging.
jamadden Oct 7, 2021
33c667c
Looks like my cleanup isn't running?
jamadden Oct 7, 2021
831ed82
Try harder to wait for active threads to die.
jamadden Oct 7, 2021
bef2234
More work on the race conditions seen on GHA
jamadden Oct 7, 2021
291c7a1
Make waiting for the greenlet cleanup much more deterministic.
jamadden Oct 7, 2021
893844a
Always use RAII/stack-based locking/unlocking.
jamadden Oct 12, 2021
e342bda
Enable exception handling for old MSVC
jamadden Oct 12, 2021
eb61b4c
Whoops, can't enable PYTHONFAULTHANDLER twice. GHA complains.
jamadden Oct 12, 2021
cf3637d
Fix a double-addressing issue.
jamadden Oct 12, 2021
7d85029
Add change note.
jamadden Oct 12, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 47 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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:

Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ greenlet.egg-info/
__pycache__/
/.ropeproject/
/MANIFEST
benchmarks/*.json
130 changes: 130 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
@@ -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:
19 changes: 14 additions & 5 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
==================
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,6 +29,7 @@ include *.cfg
include *.py
include *.ini
include .clang-format
include .pylintrc

recursive-include appveyor *.cmd
recursive-include appveyor *.ps1
Expand Down
8 changes: 5 additions & 3 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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\";}"
Expand Down