15.12.2021 22:39, John Snow wrote:
> asyncio can complain *very* loudly if you forget to back out of things
> gracefully before the garbage collector starts destroying objects that
> contain live references to asyncio Tasks.
>
> The usual fix is just to remember to call aqmp.disconnect(), but for the
> sake of the legacy wrapper and quick, one-off scripts where a graceful
> shutdown is not necessarily of paramount imporance, add a courtesy
> cleanup that will trigger prior to seeing screenfuls of confusing
> asyncio tracebacks.
>
> Note that we can't *always* save you from yourself; depending on when
> the GC runs, you might just seriously be out of luck. The best we can do
> in this case is to gently remind you to clean up after yourself.
>
> (Still much better than multiple pages of incomprehensible python
> warnings for the crime of forgetting to put your toys away.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> python/qemu/aqmp/legacy.py | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
> index 9e7b9fb80b..2ccb136b02 100644
> --- a/python/qemu/aqmp/legacy.py
> +++ b/python/qemu/aqmp/legacy.py
> @@ -16,6 +16,8 @@
> import qemu.qmp
> from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT
>
> +from .error import AQMPError
> +from .protocol import Runstate
> from .qmp_client import QMPClient
>
>
> @@ -136,3 +138,19 @@ def settimeout(self, timeout: Optional[float]) -> None:
>
> def send_fd_scm(self, fd: int) -> None:
> self._aqmp.send_fd_scm(fd)
> +
> + def __del__(self) -> None:
> + if self._aqmp.runstate == Runstate.IDLE:
> + return
> +
> + if not self._aloop.is_running():
> + self.close()
As I understand, if close() was called by hand, runstate should already be IDLE, and we don't call close() twice here?
Right ... calling close() twice is actually OK (the second one just does nothing) but what I am avoiding here is this scenario:
- close() was already called
- garbage collection runs while the event loop is running
- we should therefore NOT raise an exception.
The problem is that even if a second close() does nothing, we are not able to call into it if we're already inside of an active event loop -- so in order to achieve the no-op behavior from the GC context, I need to manually check the runstate to determine if there /WILL/ be a problem when the GC runs. If it's idle, there definitely won't be.
> + else:
> + # Garbage collection ran while the event loop was running.
> + # Nothing we can do about it now, but if we don't raise our
> + # own error, the user will be treated to a lot of traceback
> + # they might not understand.
> + raise AQMPError(
> + "QEMUMonitorProtocol.close()"
> + " was not called before object was garbage collected"
> + )
>
weak, as I'm far from understanding aqmp library:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
thanks!