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

gh-117398: gh-119655: datetime: Init static state once & don't free it #119662

Merged
merged 3 commits into from
May 28, 2024

Conversation

encukou
Copy link
Member

@encukou encukou commented May 28, 2024

if (PyModule_Add(module, "datetime_CAPI", capsule) < 0) {
PyMem_Free(capi);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not correct to free capi here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capi is static, we should not free it.

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 7009 to 7020
PyMem_Free(capi);
goto error;
}
// (capsule == NULL) {is handled by PyModule_Add
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo:

    // (capsule == NULL) is handled by PyModule_Add.

@ericsnowcurrently
Copy link
Member

I'll take care of backporting this to 3.13 (by adding it into gh-119641).

Modules/_datetimemodule.c Outdated Show resolved Hide resolved
@encukou encukou enabled auto-merge (squash) May 28, 2024 17:06
@encukou encukou merged commit a89fc26 into python:main May 28, 2024
34 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 FreeBSD 3.x has failed when building commit a89fc26.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1223/builds/3218) and take a look at the build logs.
  4. Check if the failure is related to this commit (a89fc26) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1223/builds/3218

Failed tests:

  • test_ssl

Failed subtests:

  • test_get_server_certificate_ipv6 - test.test_ssl.NetworkedTests.test_get_server_certificate_ipv6

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/buildbot/buildarea/3.x.ware-freebsd/build/Lib/test/test_ssl.py", line 2225, in test_get_server_certificate_ipv6
    _test_get_server_certificate(self, 'ipv6.google.com', 443)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/3.x.ware-freebsd/build/Lib/test/test_ssl.py", line 2230, in _test_get_server_certificate
    pem = ssl.get_server_certificate((host, port))
  File "/buildbot/buildarea/3.x.ware-freebsd/build/Lib/ssl.py", line 1514, in get_server_certificate
    with create_connection(addr, timeout=timeout) as sock:
         ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/3.x.ware-freebsd/build/Lib/socket.py", line 828, in create_connection
    for res in getaddrinfo(host, port, 0, SOCK_STREAM):
               ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/3.x.ware-freebsd/build/Lib/socket.py", line 963, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
               ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
socket.gaierror: [Errno 1] Address family for hostname not supported

ericsnowcurrently pushed a commit to ericsnowcurrently/cpython that referenced this pull request May 28, 2024
…don't free it (pythonGH-119662)

- While datetime uses global state, only initialize it once.
- While `capi` is static, don't free it (thanks @neonene in https://github.com/python/cpython/pull/119641/files#r1616710048)
@erlend-aasland
Copy link
Contributor

Thanks!

@encukou encukou deleted the datetime_state_init_once branch May 30, 2024 08:55
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…don't free it (pythonGH-119662)

- While datetime uses global state, only initialize it once.
- While `capi` is static, don't free it (thanks @neonene in https://github.com/python/cpython/pull/119641/files#r1616710048)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants