gh-116810: fix memory leak in ssl module #123249

jeffvanvoorst · 2024-08-23T02:25:46Z

PySSL_get_session leaks a session object each time it is called. For programs like cherrypy (cheroot), a leak occurs with each handled web request.

Issue: getting ssl.SSLSocket.session brings to memory leak #116810

PySSL_get_session leaks a session object each time it is called. For programs like cherrypy (cheroot), a leak occurs with each handled web request.

cpython-cla-bot · 2024-08-23T02:25:49Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2024-08-23T02:25:52Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Misc/NEWS.d/next/Library/2024-08-23-15-49-10.gh-issue-116810.QLBUU8.rst

webknjaz · 2024-08-24T00:47:01Z

Modules/_ssl.c

- session = SSL_get1_session(self->ssl);
- if (session == NULL) {
- Py_RETURN_NONE;
- }


Judging by no test failures, this probably does not have any coverage. Any chance of including the tests in this PR?

I can check the tests this week. I doubt I will be able to finish that work today.

Since this change still results in returning a valid copy of the ssl session, I doubt tests would fail.

Yeah, agreeing there. I think it would be difficult to test this. With that being said, are there any existing tests for checking that the session attribute is valid? If not, then this PR should add them (otherwise, I think this looks good).

I had some life issues come up. I will try to get back to this in the next few weeks.

There's a cacophony of mistakes here on the CPython side. Let me try to correct things:

I don't really understand how this code was nor always a memory leak in the past.

The memory leak seems to have been introduced by 39258d3. _ssl_session_dup returns an SSL_SESSION that needs to be freed by someone and then you all just immediately clobber the variable with something else. But test_ssl.py clearly accesses session. Does CPython have no way to catch memory leaks in its tests? Might I suggest you all look into LeakSanitizer?

Anyway, the problem with that commit is that you all turned #if ... #else ... #endif into including both branches instead of just one.

That commit not only introduced a memory leak, but it also mostly broke your workaround. I haven't tried it, but I suspect you would see this if you uncleanly destroyed the other connection before calling set_session instead of after. Now, I see uncleanly because it seems CPython never followed up on openssl/openssl#1550 (comment) because you all have been misunderstanding the nature of the workaround. It was not working around an OpenSSL bug, but a CPython bug / broken expectation.

Based on my understanding of https://docs.openssl.org/master/man3/SSL_get_session/ I believe this will work, but it does raise the question of why we have the SSL_get0_session _ssl_session_dup combo in the first place; with modern OpenSSL 3 why would this SSL_get1_session not just do what we wanted?

That is not what is going on here. There were, as far as I know, no changes in OpenSSL 3 relating to this.

SSL_get0_session _ssl_session_dup and SSL_get1_session do not and have never same thing in any version of OpenSSL. SSL_get0_session and SSL_get1_session are the exact same accessor. Just one of them increments the refcount for you and the other doesn't. _ssl_session_dup is a (misguided) CPython function to make a deep copy of the SSL_SESSION object.

Although the code was, prior to the regression, gated on a OPENSSL_VERSION_1_1 ifdef, that does not imply was specific to OpenSSL 1.1.x. OPENSSL_VERSION_1_1 is a CPython-invented macro (that squats OpenSSL's namespace) which means > 1.1. That is, OpenSSL 3 was also OPENSSL_VERSION_1_1:

#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && !defined(LIBRESSL_VERSION_NUMBER) # define OPENSSL_VERSION_1_1 1 # define PY_OPENSSL_1_1_API 1 #endif

While I have not tested it (have you all?), based on openssl/openssl#1550 (comment), I highly suspect that CPython "needs" the workaround in OpenSSL 3.x too. I say "needs" because...

I think the issue @.davidben was saying Python has in there was describing more the way Python is interacting with the available APIs (which at the time had to span a wide range of OpenSSL versions) and not using some other features instead.

No, that's not what I was saying in openssl/openssl#1550 (comment). See the immediately preceding comment:

FYI, I poked at this briefly and don't see any evidence that SSL_shutdown is being called by Python. It seems the pseudocode in the bug report is wrong and this is a Python bug.

The bug report was very confused because the minimal reproducer that CPython provided did not actually reflect what CPython does. With all the meandering with the bad reproducer, I suspect it was very confusing. But the CPython folks on the bug never followed up, so my past self assumed you all understood, were planning on fixing it, and I didn't have time to check on you all. 😞

What is actually going on here is that OpenSSL will invalidate sessions on connection destruction if you haven't called SSL_shutdown first. To be honest, I don't think this is good behavior, but it has been OpenSSL's intentional behavior since day 1. I believe this is descended from this guidance:

close_notify
This message notifies the recipient that the sender will not send
any more messages on this connection. The session becomes
unresumable if any connection is terminated without proper
close_notify messages with level equal to warning.

https://www.rfc-editor.org/rfc/rfc2246.html#section-7.2.1

Now, I personally think this behavior is unhelpful. If a connection just got closed, that doesn't make the old key material broken or anything. Also if we were getting any security properties out of this, it wouldn't work anyway. There are loads of cases where you cannot reliably knock out the other session.

Stateless resumption ("session tickets" in TLS 1.2) cannot possibly invalidate the session on the server

Maybe your application serializes the session, e.g., on disk, and then deserializes it later

Interestingly, OpenSSL doesn't actually look for a full bidirectional shutdown to preserve your session. It only looks for whether you attempted to send close_notify. Kind of pointless, to be honest, but IMO this whole thing is pointless.

TLS 1.1, incidentally, already softened this recommendation:

close_notify
This message notifies the recipient that the sender will not send
any more messages on this connection. Note that as of TLS 1.1,
failure to properly close a connection no longer requires that a
session not be resumed. This is a change from TLS 1.0 to conform
with widespread implementation practice.

https://www.rfc-editor.org/rfc/rfc4346.html#section-7.2.1

But OpenSSL retains this requirement, for better or worse. This requirement was not new in 1.1.x, nor was it removed in 3.x. The only reason you all needed to work around it later on is that, due to other quirks of how CPython was using the API (arguably incorrectly), as detailed in my comment on the OpenSSL issue, CPython was sneaking through a gap in this OpenSSL policy. OpenSSL fixed that gap, and now CPython was affected.

Now, contrary to the claim in openssl/openssl#1550 (comment), CPython did not call SSL_shutdown. You all have a separate method to exchange close_notify and weren't calling it in that test.

So, what to do given this? I think you have two options:

Agree with OpenSSL's policy and pass it up to the callers. Without calling SSLSocket.shutdown(), sessions get invalidated. Callers have to call the shutdown method if they care. Fix your tests to reflect this.

Disagree with OpenSSL's policy and effectively turn it off. To do that, call SSL_shutdown when destroying an SSL. Now, that will cause OpenSSL to do I/O, so you might want to SSL_set_quiet_shutdown. Alternatively, there is an SSL_set_shutdown API to just fake a shutdown state.

In either case, you can remove the workaround because this workaround has been self-inflicted from day one.

If you pick option 2, please write some clear comments in the code so the next time this happens, I don't have to write another essay for you all. :-P

The OpenSSL 1.1.1 fork's I do think we should care somewhat about are BoringSSL and it's AWS-LC derivative. @WillChilds-Klein and @davidben can represent what those might need for these specific APIs.

So, per the above, this question is based on several incorrect premises:

BoringSSL is not an OpenSSL 1.1.1 fork. We actually diverged from OpenSSL 1.0.2. We just report a version of 1.1.1 because that's what's most convenient for compatibility.

This issue has nothing to do with OpenSSL 1.1.1. I expect you all still "need" this workaround in OpenSSL 3.x too.

This issue has nothing to do with these specific APIs

Since this "workaround" is really a workaround for CPython's own mishaps, if you all do the thing I suggest above, it should work just fine in BoringSSL.

But also this is all moot in BoringSSL because BoringSSL does not invalidate sessions on SSL_free in the first place. We removed it because it's just kinda silly.

This is where OpenSSL documents their behavior, by the way:

A session will be automatically removed from the session cache and marked as non-resumable if the connection is not closed down cleanly, e.g. if a fatal error occurs on the connection or SSL_shutdown(3) is not called prior to SSL_free(3).

https://docs.openssl.org/master/man3/SSL_get_session/#notes:~:text=A session will be automatically removed from the session cache and marked as non-resumable if the connection is not closed down cleanly, e.g. if a fatal error occurs on the connection or SSL_shutdown(3) is not called prior to SSL_free(3).

For completeness, there were two different bits of spec text that led to this OpenSSL behavior, and I only cited one of them. The bits I cited were about unclean shutdown and removed very early. Fatal error lasted all the way to TLS 1.2:

Error handling in the TLS Handshake protocol is very simple. When an
error is detected, the detecting party sends a message to the other
party. Upon transmission or receipt of a fatal alert message, both
parties immediately close the connection. Servers and clients MUST
forget any session-identifiers, keys, and secrets associated with a
failed connection. Thus, any connection terminated with a fatal
alert MUST NOT be resumed.

https://www.rfc-editor.org/rfc/rfc5246.html#section-7.2.2

It wasn't removed until TLS 1.3, in tlswg/tls13-spec@dc668dc, for the same reasons. In practice, you cannot actually do this consistently, for the reasons I listed above.

Thanks!

the problem with that commit is that you all turned #if ... #else ... #endif into including both branches instead of just one.

Hah, that explains the code smell here. I'd only been glancing at current version of the code in git blame view to try and understand what this was even for without following the individual commits (git blame doing its job and not exposing history of merely deleted lines)... I agree completely on getting rid of that _ssl_session_dup hack. (and more, but we need to examine the SOTW and pick an amenable way out of the past API sins in here)

Yeah, definitely makes sense to just start with something minimal (probably this PR) to fix the immediate leak. I think probably the thing to do is:

This PR

Remove _ssl_session_dup and confirm tests indeed now fail in OpenSSL 3.x. My expectation is that they'll still fail, but I've no idea if there were other changes that further complicated this mess.

No-op the goofy OpenSSL behavior. Probably something like SSL_set_quiet_shutdown(ssl); SSL_shutdown(ssl); SSL_free(ssl); would work? Confirm the tests now pass again.

Calling SSL_set_shutdown in the destructor sounds fine to me. Is there a reason to believe that the SSL_set_quiet_shutdown(ssl); SSL_shutdown(ssl); dance would be better?

Oh and thanks for the detailed explanation @davidben .

…LBUU8.rst Co-authored-by: Peter Bierma <[email protected]>

vstinner · 2024-09-18T16:20:23Z

I confirm that the change works as expected: it fixes a significant memory leak.

I modified the reproducer to dump the RSS memory usage on Linux:

import os
import socket
import ssl
import gc

host = '142.251.163.99' # some server we can connect with https as example
port = 443

session = None
context = ssl._create_unverified_context(protocol=ssl.PROTOCOL_TLSv1_2)
with socket.create_connection((host, port)) as sock:
    ssock = context.wrap_socket(sock, server_hostname=host, session=session)
    with ssock:
        for i in range(300000):
            session = ssock.session
            if (i % 1000) == 0:
                gc.collect()
                os.system(f"grep ^VmRSS /proc/{os.getpid()}/status")

I can easily reproduce the bug:

VmRSS:	   19840 kB
VmRSS:	   26496 kB
VmRSS:	   33280 kB
VmRSS:	   40064 kB
VmRSS:	   46848 kB
VmRSS:	   53632 kB
VmRSS:	   60416 kB
VmRSS:	   67200 kB
VmRSS:	   74112 kB
VmRSS:	   80896 kB
VmRSS:	   87680 kB
VmRSS:	   94464 kB
VmRSS:	  101248 kB
(...)

With the PR, there is no leak anymore:

VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
VmRSS:	   20436 kB
(...)

vstinner

LGTM

vstinner · 2024-09-18T16:22:42Z

@gpshead @pitrou: Do you want to review this change? (I don't recall if @pitrou worked on ssl or not, ignore me if I'm wrong.)

pitrou · 2024-09-18T21:09:12Z

Yes, I worked on ssl, but that was long ago.

Misc/NEWS.d/next/Library/2024-08-23-15-49-10.gh-issue-116810.QLBUU8.rst

pitrou · 2024-09-18T21:14:23Z

Modules/_ssl.c

- session = SSL_get1_session(self->ssl);
- if (session == NULL) {
- Py_RETURN_NONE;
- }


I don't think it's possible to test anything, other than manually checking the leak is fixed as @vstinner did.

The issue was apparently that we were first duplicating the session using _ssl_session_dup, but then we immediately forgot the duplicated session by calling SSL_get1_session.

pitrou · 2024-09-18T21:15:27Z

Modules/_ssl.c

- session = SSL_get1_session(self->ssl);
- if (session == NULL) {
- Py_RETURN_NONE;
- }


That said, a potential alternate fix would be to remove the SSL_get0_session _ssl_session_dup dance and keep the SSL_get1_session call. @jeffvanvoorst Can you try to see if it works?

…tirely

pitrou · 2024-09-19T19:48:46Z

Ok, I've tried to removed the session duplication entirely and it works fine locally. I've pushed the change and we can wait for CI.

bedevere-bot · 2024-09-19T19:49:05Z

🤖 New build scheduled with the buildbot fleet by @pitrou for commit 46126b4 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot · 2024-09-19T20:07:36Z

🤖 New build scheduled with the buildbot fleet by @pitrou for commit 5ced40b 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

pitrou · 2024-09-19T20:14:08Z

It doesn't leak memory anymore: the fix works as expected.

It looks way faster!

Here as well. The session duplication hack was serializing the session object to ASN1, which is apparently quite expensive (looking at the i2d_SSL_SESSION source code, there's a lot of stuff being serialized).

vstinner

LGTM.

I'm not an OpenSSL expert but I trust other developers who were involved in this large change :-)

gpshead

I think we're still in "2. Disagree with OpenSSL's policy and effectively turn it off." state with this PR as is due to SSL_set_shutdown being used to mark it as "shutdown" so that the OpenSSL implementation does cache it? There's a nice comment at least. :)

My main goal was get rid of the obvious memory leak. I left the set_session code and obviously gross slow serialize/deserialized stuff in place last night because simply removing that without figuring out the shutdown bits did lead to test failures. Thanks for picking that up and doing the rest!

gpshead · 2024-09-19T21:15:24Z

Lets wait until @sethmlarson is back online (currently traveling) and can look at this before merging.

It isn't going to make 3.13.0 as it clearly is not a blocker (given 3.10 and 3.11 have always had this flaw and it was only reported this year). I gather session caching for clients is not super common.

Modules/_ssl.c

sethmlarson

Thanks @davidben for all the explanation, this change LGTM.

miss-islington-app · 2024-09-30T19:43:53Z

Thanks @jeffvanvoorst for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the :attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and write access to said property by no longer unnecessarily cloning session objects via serialization. (cherry picked from commit 7e7223e) Co-authored-by: Jeffrey R. Van Voorst <[email protected]> Co-authored-by: blurb-it[bot] <43283697 blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

bedevere-app · 2024-09-30T19:44:01Z

GH-124800 is a backport of this pull request to the 3.13 branch.

Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the :attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and write access to said property by no longer unnecessarily cloning session objects via serialization. (cherry picked from commit 7e7223e) Co-authored-by: Jeffrey R. Van Voorst <[email protected]> Co-authored-by: blurb-it[bot] <43283697 blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

bedevere-app · 2024-09-30T19:44:06Z

GH-124801 is a backport of this pull request to the 3.12 branch.

gh-116810: fix memory leak in ssl module (GH-123249) Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the :attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and write access to said property by no longer unnecessarily cloning session objects via serialization. (cherry picked from commit 7e7223e) Co-authored-by: Jeffrey R. Van Voorst <[email protected]> Co-authored-by: blurb-it[bot] <43283697 blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

jeffvanvoorst · 2024-10-01T18:02:04Z

Thanks everyone. I appreciate the cpython community support.

gh-116810: fix memory leak in ssl module (GH-123249) Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the :attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and write access to said property by no longer unnecessarily cloning session objects via serialization. (cherry picked from commit 7e7223e) Co-authored-by: Jeffrey R. Van Voorst <[email protected]> Co-authored-by: blurb-it[bot] <43283697 blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

pythongh-116810: fixed memory leak ssl module

119d33a

PySSL_get_session leaks a session object each time it is called. For programs like cherrypy (cheroot), a leak occurs with each handled web request.

bedevere-app bot mentioned this pull request Aug 23, 2024

getting ssl.SSLSocket.session brings to memory leak #116810

Closed

bedevere-app bot added the awaiting review label Aug 23, 2024

blurb-it bot and others added 3 commits August 23, 2024 15:49

📜🤖 Added by blurb_it.

c98be51

Merge branch 'main' into ssl-bugfix-116810

e12cb28

Merge branch 'main' into ssl-bugfix-116810

13196b4

ZeroIntensity reviewed Aug 23, 2024

View reviewed changes

Misc/NEWS.d/next/Library/2024-08-23-15-49-10.gh-issue-116810.QLBUU8.rst Outdated Show resolved Hide resolved

webknjaz mentioned this pull request Aug 24, 2024

Memory leak when using BuiltinSSLAdapter cherrypy/cheroot#647

Open

3 tasks

webknjaz reviewed Aug 24, 2024

View reviewed changes

jeffvanvoorst and others added 4 commits August 26, 2024 10:29

Update Misc/NEWS.d/next/Library/2024-08-23-15-49-10.gh-issue-116810.Q…

8fff948

…LBUU8.rst Co-authored-by: Peter Bierma <[email protected]>

Merge branch 'main' into ssl-bugfix-116810

507266e

Merge branch 'main' into ssl-bugfix-116810

c66f537

Merge branch 'main' into ssl-bugfix-116810

45df217

vstinner approved these changes Sep 18, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Sep 18, 2024

vstinner added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Sep 18, 2024

gpshead assigned gpshead and sethmlarson Sep 18, 2024

pitrou reviewed Sep 18, 2024

View reviewed changes

pitrou changed the title ~~gh-116810: fixed memory leak in ssl module~~ gh-116810: fix memory leak in ssl module Sep 18, 2024

gpshead added extension-modules C modules in the Modules dir topic-SSL labels Sep 19, 2024

Make the NEWS entry more detailed.

537c5bb

Call SSL_set_shutdown before SSL_free, remove session deecopy hack en…

46126b4

…tirely

pitrou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 19, 2024

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 19, 2024

pitrou added 2 commits September 19, 2024 21:52

Remove stale comment

7131837

Merge branch 'main' into ssl-bugfix-116810

5ced40b

pitrou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 19, 2024

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 19, 2024

vstinner approved these changes Sep 19, 2024

View reviewed changes

expands news to mention the lack of internal serialization speedup.

96ab92b

gpshead approved these changes Sep 19, 2024

View reviewed changes

davidben reviewed Sep 20, 2024

View reviewed changes

Modules/_ssl.c Outdated Show resolved Hide resolved

per davidben, lets not clear a shutdown bit.

cb415bc

sethmlarson approved these changes Sep 30, 2024

View reviewed changes

gpshead merged commit 7e7223e into python:main Sep 30, 2024
37 checks passed

bedevere-app bot removed the awaiting merge label Sep 30, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 30, 2024

bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Sep 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-116810: fix memory leak in ssl module #123249

gh-116810: fix memory leak in ssl module #123249

jeffvanvoorst commented Aug 23, 2024 •

edited by bedevere-app bot

Loading

cpython-cla-bot bot commented Aug 23, 2024 •

edited

Loading

bedevere-app bot commented Aug 23, 2024

webknjaz Aug 24, 2024

jeffvanvoorst Aug 26, 2024

jeffvanvoorst Aug 26, 2024

ZeroIntensity Aug 27, 2024

jeffvanvoorst Sep 11, 2024

davidben Sep 19, 2024

davidben Sep 19, 2024

gpshead Sep 19, 2024

davidben Sep 19, 2024

pitrou Sep 19, 2024

vstinner commented Sep 18, 2024

vstinner left a comment

vstinner commented Sep 18, 2024

pitrou commented Sep 18, 2024

pitrou Sep 18, 2024

pitrou Sep 18, 2024

pitrou commented Sep 19, 2024

bedevere-bot commented Sep 19, 2024

bedevere-bot commented Sep 19, 2024

pitrou commented Sep 19, 2024

vstinner left a comment

gpshead left a comment

gpshead commented Sep 19, 2024

sethmlarson left a comment

miss-islington-app bot commented Sep 30, 2024

bedevere-app bot commented Sep 30, 2024

bedevere-app bot commented Sep 30, 2024

jeffvanvoorst commented Oct 1, 2024

gh-116810: fix memory leak in ssl module #123249

gh-116810: fix memory leak in ssl module #123249

Conversation

jeffvanvoorst commented Aug 23, 2024 • edited by bedevere-app bot Loading

cpython-cla-bot bot commented Aug 23, 2024 • edited Loading

bedevere-app bot commented Aug 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Sep 18, 2024

vstinner left a comment

Choose a reason for hiding this comment

vstinner commented Sep 18, 2024

pitrou commented Sep 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitrou commented Sep 19, 2024

bedevere-bot commented Sep 19, 2024

bedevere-bot commented Sep 19, 2024

pitrou commented Sep 19, 2024

vstinner left a comment

Choose a reason for hiding this comment

gpshead left a comment

Choose a reason for hiding this comment

gpshead commented Sep 19, 2024

sethmlarson left a comment

Choose a reason for hiding this comment

miss-islington-app bot commented Sep 30, 2024

bedevere-app bot commented Sep 30, 2024

bedevere-app bot commented Sep 30, 2024

jeffvanvoorst commented Oct 1, 2024

jeffvanvoorst commented Aug 23, 2024 •

edited by bedevere-app bot

Loading

cpython-cla-bot bot commented Aug 23, 2024 •

edited

Loading