gh-140691: urllib.request: Close FTP control socket if data socket can't connect #140835

encukou · 2025-10-31T14:26:56Z

Issue: Unclosed socket in urllib when ftp request times out after connect #140691

Co-Authored-By: codenamenam <[email protected]>

encukou · 2025-10-31T14:29:48Z

Lib/test/test_urllib2net.py

-                        if expected_err:
-                            msg = ("Didn't get expected error(s) %s for %s %s, got %s: %s" %
-                                   (expected_err, url, req, type(err), err))
-                            self.assertIsInstance(err, expected_err, msg)
-                        else:
-                            raise


(The new context (assertRaises) handles the expected error; printing information is nowadays covered by subTest)

bedevere-bot · 2025-10-31T15:01:37Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit 3cbf28b 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge

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

cmaloney

Implementation looks correct and test covers the case well

Lib/test/test_urllib2net.py

cmaloney · 2025-11-01T02:27:04Z

The PPPC64LE RHEL8 Refleaks PR build seems to have found a distinct but similar unclosed socket in exception case:

test_ftp (test.test_urllib2net.OtherNetworkTests.test_ftp) ... Warning -- Unraisable exception
Exception ignored while finalizing socket <socket.socket fd=3, family=2, type=1, proto=6, laddr=('147.229.8.193', 48118), raddr=('68.183.26.59', 21)>:
Traceback (most recent call last):
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1822, in retrfile
    self.ftp.cwd(file)
    ~~~~~~~~~~~~^^^^^^
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 619, in cwd
    return self.voidcmd(cmd)
           ~~~~~~~~~~~~^^^^^
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 286, in voidcmd
    return self.voidresp()
           ~~~~~~~~~~~~~^^
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 259, in voidresp
    resp = self.getresp()
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 254, in getresp
    raise error_perm(resp)
ftplib.error_perm: 550 Failed to change directory.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1547, in ftp_open
    fp, retrlen = fw.retrfile(file, type)
                  ~~~~~~~~~~~^^^^^^^^^^^^
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1824, in retrfile
    raise URLError('ftp error: %r' % reason) from reason
urllib.error.URLError: <urlopen error ftp error: error_perm('550 Failed to change directory.')>

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_urllib2net.py", line 23, in _retry_thrice
    return func(*args, **kwargs)
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 487, in open
    response = self._open(req, data)
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 504, in _open
    result = self._call_chain(self.handle_open, protocol, protocol +
                              '_open', req)
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 464, in _call_chain
    result = func(*args)
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1560, in ftp_open
    raise URLError(f"ftp error: {exp}") from exp
urllib.error.URLError: <urlopen error ftp error: <urlopen error ftp error: error_perm('550 Failed to change directory.')>>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_urllib2net.py", line 25, in _retry_thrice
    last_exc = e
    ^^^^^^^^
ResourceWarning: unclosed <socket.socket fd=3, family=2, type=1, proto=6, laddr=('147.229.8.193', 48118), raddr=('68.183.26.59', 21)>
ok

build:https://buildbot.python.org/#/builders/353/builds/2319

encukou · 2025-11-03T13:15:15Z

Thank you for the review!

Hmm, how did you get the traceback? The logs show check_no_warnings comparing two lists of warnings.

encukou · 2025-11-03T13:53:51Z

!buildbot PPC64LE.Fedora.Stable

bedevere-bot · 2025-11-03T13:53:54Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit 57634d3 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge

The command will test the builders whose names match following regular expression: PPC64LE.Fedora.Stable

The builders matched are:

PPC64LE Fedora Stable LTO PR
PPC64LE Fedora Stable Clang Installed PR
PPC64LE Fedora Stable LTO + PGO PR
PPC64LE Fedora Stable Clang PR
PPC64LE Fedora Stable PR
PPC64LE Fedora Stable Refleaks PR

encukou · 2025-11-03T16:41:05Z

!buildbot PPC64LE.Fedora.Stable.Refleaks

bedevere-bot · 2025-11-03T16:41:09Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit d329345 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge

The command will test the builders whose names match following regular expression: PPC64LE.Fedora.Stable.Refleaks

The builders matched are:

PPC64LE Fedora Stable Refleaks PR

cmaloney · 2025-11-03T18:17:03Z

Hmm, how did you get the traceback? The logs show check_no_warnings comparing two lists of warnings.

In the linked buildbot log expanded the test step, then used the "Raw" button next to stdio, is in the full log.

encukou · 2025-11-04T13:19:39Z

Ah, I missed that it only appeared on that specific run; the other runs are failing in a different way.

encukou · 2025-11-04T13:19:48Z

!buildbot PPC64LE.Fedora.Stable.Refleaks

bedevere-bot · 2025-11-04T13:19:51Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit cd9d5f8 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge

The command will test the builders whose names match following regular expression: PPC64LE.Fedora.Stable.Refleaks

The builders matched are:

PPC64LE Fedora Stable Refleaks PR

encukou · 2025-11-04T14:36:39Z

@vstinner, if you have a good way to reproduce this PR failing refleak tests on a PPC64LE machine, it would be appreciated. I couldn't reproduce it in QEMU/podman.

bedevere-bot · 2025-11-04T21:04:52Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit d20a451 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge

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

encukou · 2025-11-05T10:51:37Z

To unblock failing buildbots, I'll merge with a skip on PPC64LE. I'm pretty sure this doesn't break PPC64LE -- it only fails in the new test, and that can be solved in a new PR.

miss-islington-app · 2025-11-14T13:23:04Z

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

…ket can't connect (pythonGH-140835) (cherry picked from commit f2bce51) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: codenamenam <[email protected]>

bedevere-app · 2025-11-14T13:23:12Z

GH-141555 is a backport of this pull request to the 3.14 branch.

…cket can't connect (GH-140835) (GH-141555) (cherry picked from commit f2bce51) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: codenamenam <[email protected]>

miss-islington-app · 2025-11-17T11:28:21Z

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2025-11-17T11:28:24Z

Sorry, @encukou, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker f2bce51b984f52db14d90f7bbd0b7df00b7c5637 3.13

…ket can't connect (pythonGH-140835) Co-authored-by: codenamenam <[email protected]> (cherry picked from commit f2bce51)

bedevere-app · 2025-11-17T13:34:17Z

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

…cket can't connect (GH-140835) (GH-141657) (cherry picked from commit f2bce51) Co-authored-by: codenamenam <[email protected]>

…ket can't connect (pythonGH-140835) Co-authored-by: codenamenam <[email protected]>

encukou and others added 4 commits October 31, 2025 15:01

Add a test for unclosed socket

bd3c98d

Close FTP control connection if transfer connection fails

b0b636a

Co-Authored-By: codenamenam <[email protected]>

Remove closed connections from cache

97c2336

Add blurb

3cbf28b

bedevere-app bot mentioned this pull request Oct 31, 2025

Unclosed socket in urllib when ftp request times out after connect #140691

Open

bedevere-app bot added the awaiting core review label Oct 31, 2025

encukou commented Oct 31, 2025

View reviewed changes

encukou requested a review from cmaloney October 31, 2025 14:30

encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 31, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 31, 2025

cmaloney approved these changes Oct 31, 2025

View reviewed changes

Lib/test/test_urllib2net.py Show resolved Hide resolved

Lib/test/test_urllib2net.py Show resolved Hide resolved

encukou added 3 commits November 3, 2025 13:54

Add the walltime resource

20703c4

Merge in the main branch

5ee397e

Close non-keepalive connections when discarding them

57634d3

Show WarningMessage details in __repr__

d329345

encukou added 2 commits November 4, 2025 12:51

Investigating: turn warnings to errors to get tracebacks

9ca07e8

import warnings

cd9d5f8

Don't retry -- network errors skip the test, anyway

c477938

encukou added 2 commits November 4, 2025 16:54

Don't turn warnings into errors

432bac2

Skip new test on on PPC64LE for now

d20a451

encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 4, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 4, 2025

encukou merged commit f2bce51 into python:main Nov 5, 2025
115 of 116 checks passed

bedevere-app bot removed the awaiting core review label Nov 5, 2025

encukou deleted the ftp-warnings branch November 5, 2025 10:52

encukou added the needs backport to 3.14 bugs and security fixes label Nov 14, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Nov 14, 2025

encukou added the needs backport to 3.13 bugs and security fixes label Nov 17, 2025

miss-islington-app bot assigned encukou Nov 17, 2025

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 17, 2025

encukou added a commit that referenced this pull request Nov 18, 2025

[3.13] gh-140691: urllib.request: Close FTP control socket if data so…

1bca1b4

…cket can't connect (GH-140835) (GH-141657) (cherry picked from commit f2bce51) Co-authored-by: codenamenam <[email protected]>

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025

pythongh-140691: urllib.request: Close FTP control socket if data soc…

a4a0261

…ket can't connect (pythonGH-140835) Co-authored-by: codenamenam <[email protected]>

Uh oh!

gh-140691: urllib.request: Close FTP control socket if data socket can't connect #140835

gh-140691: urllib.request: Close FTP control socket if data socket can't connect #140835

Uh oh!

Conversation

encukou commented Oct 31, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

encukou Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 31, 2025

Uh oh!

cmaloney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmaloney commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

encukou commented Nov 3, 2025

Uh oh!

encukou commented Nov 3, 2025

Uh oh!

bedevere-bot commented Nov 3, 2025

Uh oh!

encukou commented Nov 3, 2025

Uh oh!

bedevere-bot commented Nov 3, 2025

Uh oh!

cmaloney commented Nov 3, 2025

Uh oh!

encukou commented Nov 4, 2025

Uh oh!

encukou commented Nov 4, 2025

Uh oh!

bedevere-bot commented Nov 4, 2025

Uh oh!

encukou commented Nov 4, 2025

Uh oh!

bedevere-bot commented Nov 4, 2025

Uh oh!

encukou commented Nov 5, 2025

Uh oh!

Uh oh!

miss-islington-app bot commented Nov 14, 2025

Uh oh!

bedevere-app bot commented Nov 14, 2025

Uh oh!

miss-islington-app bot commented Nov 17, 2025

Uh oh!

miss-islington-app bot commented Nov 17, 2025

Uh oh!

bedevere-app bot commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

encukou commented Oct 31, 2025 •

edited by bedevere-app bot

Loading

cmaloney commented Nov 1, 2025 •

edited

Loading