diff mbox series

[09/10] python/aqmp: fix race condition in legacy.py

Message ID 20220225205948.3693480-10-jsnow@redhat.com
State New
Headers show
Series Python: Fix qmp race condition on accept() | expand

Commit Message

John Snow Feb. 25, 2022, 8:59 p.m. UTC
With all of that refactoring out of the way, this is extraordinarily
simple.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/legacy.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Daniel P. Berrangé March 4, 2022, 6:01 p.m. UTC | #1
On Fri, Feb 25, 2022 at 03:59:47PM -0500, John Snow wrote:
> With all of that refactoring out of the way, this is extraordinarily
> simple.

I think it'd be useful to explain in some detail what the
race condition was, because 1 year later no one will
remember what scenario we were fixing here.

> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/aqmp/legacy.py | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
John Snow March 4, 2022, 6:23 p.m. UTC | #2
On Fri, Mar 4, 2022 at 1:01 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Feb 25, 2022 at 03:59:47PM -0500, John Snow wrote:
> > With all of that refactoring out of the way, this is extraordinarily
> > simple.
>
> I think it'd be useful to explain in some detail what the
> race condition was, because 1 year later no one will
> remember what scenario we were fixing here.
>

OK, not a problem. Thanks for the reviews thus far.

[...]
diff mbox series

Patch

diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
index dca1e76ed4..cb50e60564 100644
--- a/python/qemu/aqmp/legacy.py
+++ b/python/qemu/aqmp/legacy.py
@@ -57,7 +57,7 @@  def __init__(self, address: SocketAddrT,
         self._timeout: Optional[float] = None
 
         if server:
-            self._aqmp._bind_hack(address)  # pylint: disable=protected-access
+            self._sync(self._aqmp.start_server(address))
 
     _T = TypeVar('_T')
 
@@ -90,10 +90,7 @@  def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
         self._aqmp.await_greeting = True
         self._aqmp.negotiate = True
 
-        self._sync(
-            self._aqmp.start_server_and_accept(self._address),
-            timeout
-        )
+        self._sync(self._aqmp.accept(), timeout)
 
         ret = self._get_greeting()
         assert ret is not None