python3: fix a race condition in the test_socket.testSockName test

Message ID 20220617124820.3030925-1-ross.burton@arm.com
State Accepted, archived
Commit efac044cabdbe77556a0b9903595fd602f39ac8d
Headers show
Series python3: fix a race condition in the test_socket.testSockName test | expand

Commit Message

Ross Burton June 17, 2022, 12:48 p.m. UTC
This test uses find_unused_port() which is inherently racey, so retry
it a few times before failing.

[ YOCTO #14840 ]

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 ...e-the-race-condition-in-testSockName.patch | 47 +++++++++++++++++++
 .../recipes-devtools/python/python3_3.10.4.bb |  1 +
 2 files changed, 48 insertions(+)
 create mode 100644 meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch

Comments

Alexander Kanavin June 23, 2022, 2:47 p.m. UTC | #1
I trust this will be submitted upstream right after the python CLA is
signed and approved?

Alex

On Fri, 17 Jun 2022 at 14:48, Ross Burton <ross.burton@arm.com> wrote:
>
> This test uses find_unused_port() which is inherently racey, so retry
> it a few times before failing.
>
> [ YOCTO #14840 ]
>
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
>  ...e-the-race-condition-in-testSockName.patch | 47 +++++++++++++++++++
>  .../recipes-devtools/python/python3_3.10.4.bb |  1 +
>  2 files changed, 48 insertions(+)
>  create mode 100644 meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch
>
> diff --git a/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch b/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch
> new file mode 100644
> index 00000000000..e19df08f870
> --- /dev/null
> +++ b/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch
> @@ -0,0 +1,47 @@
> +Upstream-Status: Pending
> +Signed-off-by: Ross Burton <ross.burton@arm.com>
> +
> +From 8103b90148e8768456c3ab707de105d63d9d5b20 Mon Sep 17 00:00:00 2001
> +From: Ross Burton <ross.burton@arm.com>
> +Date: Fri, 17 Jun 2022 11:53:59 +0100
> +Subject: [PATCH] Mitigate the race condition in testSockName
> +
> +find_unused_port() has an inherent race condition, but we can't use
> +bind_port() as that uses .getsockname() which this test is exercising.
> +
> +Try binding to unused ports a few times before failing.
> +---
> + Lib/test/test_socket.py | 15 +++++++++++++--
> + 1 file changed, 13 insertions(+), 2 deletions(-)
> +
> +diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
> +index c981903824..b1630d18b6 100644
> +--- a/Lib/test/test_socket.py
> ++++ b/Lib/test/test_socket.py
> +@@ -1390,10 +1390,21 @@ def testStringToIPv6(self):
> +
> +     def testSockName(self):
> +         # Testing getsockname()
> +-        port = socket_helper.find_unused_port()
> +         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> +         self.addCleanup(sock.close)
> +-        sock.bind(("0.0.0.0", port))
> ++
> ++        # Since find_unused_port() is inherently subject to race conditions, we
> ++        # call it a couple times if necessary.
> ++        for i in itertools.count():
> ++            port = socket_helper.find_unused_port()
> ++            try:
> ++                sock.bind(("0.0.0.0", port))
> ++            except OSError as e:
> ++                if e.errno != errno.EADDRINUSE or i == 5:
> ++                    raise
> ++            else:
> ++                break
> ++
> +         name = sock.getsockname()
> +         # XXX(nnorwitz): http://tinyurl.com/os5jz seems to indicate
> +         # it reasonable to get the host's addr in addition to 0.0.0.0.
> +--
> +2.25.1
> +
> diff --git a/meta/recipes-devtools/python/python3_3.10.4.bb b/meta/recipes-devtools/python/python3_3.10.4.bb
> index 6bd3a6aba84..db0c2bf9ee8 100644
> --- a/meta/recipes-devtools/python/python3_3.10.4.bb
> +++ b/meta/recipes-devtools/python/python3_3.10.4.bb
> @@ -34,6 +34,7 @@ SRC_URI = "http://www.python.org/ftp/python/${PV}/Python-${PV}.tar.xz \
>             file://0001-Lib-pty.py-handle-stdin-I-O-errors-same-way-as-maste.patch \
>             file://0001-setup.py-Do-not-detect-multiarch-paths-when-cross-co.patch \
>             file://deterministic_imports.patch \
> +           file://0001-Mitigate-the-race-condition-in-testSockName.patch \
>             "
>
>  SRC_URI:append:class-native = " \
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#167023): https://lists.openembedded.org/g/openembedded-core/message/167023
> Mute This Topic: https://lists.openembedded.org/mt/91820207/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Ross Burton June 23, 2022, 2:51 p.m. UTC | #2
> I trust this will be submitted upstream right after the python CLA is
> signed and approved?

I can’t just sign it due to employment etc etc, but yes, the process is in motion.

Ross

Patch

diff --git a/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch b/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch
new file mode 100644
index 00000000000..e19df08f870
--- /dev/null
+++ b/meta/recipes-devtools/python/python3/0001-Mitigate-the-race-condition-in-testSockName.patch
@@ -0,0 +1,47 @@ 
+Upstream-Status: Pending
+Signed-off-by: Ross Burton <ross.burton@arm.com>
+
+From 8103b90148e8768456c3ab707de105d63d9d5b20 Mon Sep 17 00:00:00 2001
+From: Ross Burton <ross.burton@arm.com>
+Date: Fri, 17 Jun 2022 11:53:59 +0100
+Subject: [PATCH] Mitigate the race condition in testSockName
+
+find_unused_port() has an inherent race condition, but we can't use
+bind_port() as that uses .getsockname() which this test is exercising.
+
+Try binding to unused ports a few times before failing.
+---
+ Lib/test/test_socket.py | 15 +++++++++++++--
+ 1 file changed, 13 insertions(+), 2 deletions(-)
+
+diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
+index c981903824..b1630d18b6 100644
+--- a/Lib/test/test_socket.py
++++ b/Lib/test/test_socket.py
+@@ -1390,10 +1390,21 @@ def testStringToIPv6(self):
+ 
+     def testSockName(self):
+         # Testing getsockname()
+-        port = socket_helper.find_unused_port()
+         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+         self.addCleanup(sock.close)
+-        sock.bind(("0.0.0.0", port))
++
++        # Since find_unused_port() is inherently subject to race conditions, we
++        # call it a couple times if necessary.
++        for i in itertools.count():
++            port = socket_helper.find_unused_port()
++            try:
++                sock.bind(("0.0.0.0", port))
++            except OSError as e:
++                if e.errno != errno.EADDRINUSE or i == 5:
++                    raise
++            else:
++                break
++
+         name = sock.getsockname()
+         # XXX(nnorwitz): http://tinyurl.com/os5jz seems to indicate
+         # it reasonable to get the host's addr in addition to 0.0.0.0.
+-- 
+2.25.1
+
diff --git a/meta/recipes-devtools/python/python3_3.10.4.bb b/meta/recipes-devtools/python/python3_3.10.4.bb
index 6bd3a6aba84..db0c2bf9ee8 100644
--- a/meta/recipes-devtools/python/python3_3.10.4.bb
+++ b/meta/recipes-devtools/python/python3_3.10.4.bb
@@ -34,6 +34,7 @@  SRC_URI = "http://www.python.org/ftp/python/${PV}/Python-${PV}.tar.xz \
            file://0001-Lib-pty.py-handle-stdin-I-O-errors-same-way-as-maste.patch \
            file://0001-setup.py-Do-not-detect-multiarch-paths-when-cross-co.patch \
            file://deterministic_imports.patch \
+           file://0001-Mitigate-the-race-condition-in-testSockName.patch \
            "
 
 SRC_URI:append:class-native = " \