On Tue, Aug 17, 2021 at 3:07 PM Niteesh G. S. <niteesh...@gmail.com> wrote:
>
>
> On Tue, Aug 17, 2021 at 10:21 AM John Snow <js...@redhat.com> wrote:
>
>>
>>
>> On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu <niteesh...@gmail.com>
>> wrote:
>>
>>
[...]
>
>>
>>> +
>>> +
>>> class App(QMPClient):
>>> - def __init__(self, address: Union[str, Tuple[str, int]]) -> None:
>>> + def __init__(self, address: Union[str, Tuple[str, int]],
>>> num_retries: int,
>>> + retry_delay: Optional[int]) -> None:
>>> urwid.register_signal(type(self), UPDATE_MSG)
>>> self.window = Window(self)
>>> self.address = address
>>> self.aloop: Optional[Any] = None # FIXME: Use more concrete
>>> type.
>>> + self.num_retries = num_retries
>>> + self.retry_delay = retry_delay
>>> + self.retry: bool = False
>>> + self.disconnecting: bool = False
>>>
>>
>> Why is this one needed again ? ...
>>
>
> A race condition occurs in protocol.py line 597
> The reason behind this is there are two disconnect calls initiated. The
> first one via kill_app
> and the second one via manage_connection when the state is set to
> disconnecting by the first call.
> One of the calls set's the state to IDLE(protocol.py:584) after it has
> finished disconnecting, meanwhile
> the second call is somehow in the process of disconnecting and assert the
> state to be in DISCONNECTING
> in protocol.py:597, which it is not since it has been set to IDLE by the
> first call.
>
> If I don't gaurd against the second call I get the following exception
>
> ------------------------------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/niteesh/development/qemu/python/.venv/bin/aqmp-tui", line
> 33, in <module>
> sys.exit(load_entry_point('qemu', 'console_scripts', 'aqmp-tui')())
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 695, in main
> app.run(args.asyncio_debug)
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 444, in run
> raise err
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 441, in run
> main_loop.run()
> File
> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py",
> line 287, in run
> self._run()
> File
> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py",
> line 385, in _run
> self.event_loop.run()
> File
> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py",
> line 1494, in run
> reraise(*exc_info)
> File
> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/compat.py",
> line 58, in reraise
> raise value
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 391, in manage_connection
> await self.disconnect()
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 312, in disconnect
> raise err
> File "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line
> 300, in disconnect
> await super().disconnect()
> File "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line
> 302, in disconnect
> await self._wait_disconnect()
> File "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line
> 583, in _wait_disconnect
> self._cleanup()
> File "/home/niteesh/development/qemu/python/qemu/aqmp/qmp_client.py",
> line 331, in _cleanup
> super()._cleanup()
> File "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line
> 597, in _cleanup
> assert self.runstate == Runstate.DISCONNECTING
> AssertionError
>
> -------------------------------------------------------------------------------------------
>
Hm, OK. I'm not sure if this is a bug on my part or not yet, I'll
investigate.
> def add_to_history(self, msg: str, level: Optional[str] = None) ->
>>> None:
>>> @@ -119,7 +132,7 @@ def _cb_inbound(self, msg: Message) -> Message:
>>> LOGGER.info('Error server disconnected before reply')
>>> urwid.emit_signal(self, UPDATE_MSG,
>>> '{"error": "Server disconnected before
>>> reply"}')
>>> - self._set_status("Server disconnected")
>>> + await self.disconnect()
>>> except Exception as err:
>>> LOGGER.error('Exception from _send_to_server: %s', str(err))
>>> raise err
>>> @@ -136,15 +149,29 @@ def kill_app(self) -> None:
>>> create_task(self._kill_app())
>>
>> Is this required? I would have hoped that after calling disconnect that
>> the state would have again changed to IDLE and you wouldn't need this
>> clause here.
>>
> After you mentioned it I too felt it was redundant. But on removing it the
> whole app freezes when trying to exit.
> I logged the state after the call to disconnect, instead of being in the
> IDLE state, it is still in DISCONNECTING state.
> I suspect this results in the constant infinite looping which doesn't give
> other coroutines a chance to run and blocks
> the event loop thus resulting in the freezing of the app. But I am not
> sure why the state isn't changing to IDLE.
>
Hmm ... That may well be a bug in AQMP then. I will investigate.
