On Thu, Dec 16, 2021 at 4:31 AM Vladimir Sementsov-Ogievskiy < vsement...@virtuozzo.com> wrote:
> 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 <js...@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 <vsement...@virtuozzo.com> > > thanks! --js
