Skip to content

Commit b545c72

Browse files
committed
gh-160561: Address review comments on the fromfd() leak fix
Remove the code comment and use a bare except, as requested. Rename the test to testFromFdError, flatten it and use os_helper.fd_count() so the descriptor count works on all platforms.
1 parent c7af374 commit b545c72

2 files changed

Lines changed: 16 additions & 21 deletions

File tree

‎Lib/socket.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -574,9 +574,7 @@ def fromfd(fd, family, type, proto=0):
574574
nfd = dup(fd)
575575
try:
576576
return socket(family, type, proto, nfd)
577-
except BaseException:
578-
# socket() rejected the arguments; the dup()'d descriptor would
579-
# otherwise leak, since nothing holds a reference to it.
577+
except:
580578
close(nfd)
581579
raise
582580

‎Lib/test/test_socket.py‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3195,25 +3195,22 @@ def testFromFd(self):
31953195
def _testFromFd(self):
31963196
self.serv_conn.send(MSG)
31973197

3198-
def testFromFdInvalidArgsDoesNotLeak(self):
3199-
# fromfd() dup()s the descriptor before socket() validates its
3200-
# arguments; a rejected call must not leave the duplicate open.
3201-
fddir = '/proc/self/fd'
3202-
if not os.path.isdir(fddir):
3203-
self.skipTest('test needs /proc/self/fd')
3198+
def testFromFdError(self):
3199+
# Make sure that the fromfd() duplicated socket is closed on error.
3200+
# In debug mode, Python test suite checks for file descriptor leaks.
32043201
fd = self.cli_conn.fileno()
3205-
base = dict(family=socket.AF_INET, type=socket.SOCK_STREAM, proto=0)
3206-
# family, type and proto are all converted to a C int, so each of them
3207-
# can fail after the dup() has already happened.
3208-
for name in base:
3209-
kwargs = dict(base, **{name: 1 << 100})
3210-
with self.subTest(name=name):
3211-
before = len(os.listdir(fddir))
3212-
with self.assertRaises(OverflowError):
3213-
socket.fromfd(fd, **kwargs)
3214-
self.assertEqual(len(os.listdir(fddir)), before)
3215-
3216-
def _testFromFdInvalidArgsDoesNotLeak(self):
3202+
fd_count_before = os_helper.fd_count()
3203+
3204+
with self.assertRaises(TypeError):
3205+
socket.fromfd(fd, "not-a-valid-family", socket.SOCK_STREAM)
3206+
with self.assertRaises(TypeError):
3207+
socket.fromfd(fd, socket.AF_UNIX, "not-a-valid-type")
3208+
with self.assertRaises(OverflowError):
3209+
socket.fromfd(fd, socket.AF_UNIX, 1 << 100)
3210+
3211+
self.assertEqual(os_helper.fd_count(), fd_count_before)
3212+
3213+
def _testFromFdError(self):
32173214
pass
32183215

32193216
def testDup(self):

0 commit comments

Comments
 (0)