Skip to content
Snippets Groups Projects
Commit edffdd10 authored by Raf Geens's avatar Raf Geens Committed by lejeune quentin
Browse files

[FIX] hw_drivers: InterfaceMetaClass violates CPython constraints and breaks import

Currently, none of the classes derived from Interface use `super`. If
it's added, for example by adding this to `SocketInterface`:

```
def __init__(self):
    super().__init__()
```

, a deprecation warning will appear in Python 3.6 and 3.7:

```
2021-04-30 13:14:25,000 24736 WARNING ? py.warnings: /home/pi/odoo/addons/hw_drivers/iot_handlers/interfaces/SocketInterface.py:11: DeprecationWarning: class not set defining 'SocketInterface' as <class 'SocketInterface.py.SocketInterface'>. Was classcell propagated to type.new?
  class SocketInterface(Interface):
```

In Python 3.8 this is no longer a warning and becomes a `RuntimeError`.

The reason this happens is that `InterfaceMetaClass` caches the results
of the `__new__` calls it does in `interfaces`. CPython's data model
specifies that when calling `__new__`, the cell for `__class__` needs to
be passed along to the parent class: https://docs.python.org/3/reference/datamodel.html#creating-the-class-object .

See https://docs.python.org/3/c-api/cell.html for a definition of what
a cell is.

Because of the caching, this doesn't happen when `__new__` is called
multiple times for the same class. In the case of the `SocketInterface`
example, `__new__` is called twice: first when it's imported directly by
`load_iot_handlers` (https://github.com/odoo/odoo/blob/841c016913a87133bf7257c62ae5f6bf7d99e06d/addons/hw_drivers/main.py#L81)
and second when `IngenicoDriver` imports it. The `__class__` cell is
different each time in that case. With the caching present, this means
the second cell isn't propagated correctly, resulting in the warning /
error.

After `load_iot_handlers` has finished, the `Manager`'s `run` method
iterates over `interfaces`, instantiating and starting the loaded
classes. Because of this, simply removing the caching appears to be
sufficient to avoid the issue while keeping the end result the same.

This issue was encountered while forward-porting a V13 PR, which adds a
`super` use to `SocketInterface`. The issue didn't occur in V13, because
the metaclass doesn't have a cache there.

I found a second problem resulting from the caching. I had noticed that
the `socket_devices` dict the `IngenicoDriver` had access to did not
have the same contents as the one the `SocketInterface` was operating
on. This means that whenever you called the `disconnect` function on the
`IngenicoDriver`, which deletes its entry from `socket_devices`, it will
always fail. The bluetooth driver seems to be impacted by the same.

It turns out the reason for that is how downloaded modules get loaded in
`load_iot_handlers`. That logic (using `exec_module`) doesn't actually
import the module, it just executes the code (registering the interfaces
and drivers as a side-effect). This means the module doesn't get cached
in sys.modules. That means that when another module (`IngenicoDriver`)
imports that same module (`SocketInterface`), Python will execute the
entire contents again, in this case effectively replacing socket_devices
with a new dict, which `IngenicoDriver` gets a reference to, while the
`SocketInterface` thread that runs is still using the old reference to
the original dict. Meaning the two are out of sync. See
https://docs.python.org/3/reference/import.html#the-module-cache for
details on how the import caching works.

The reason the classes get out of sync is because the metaclass doesn't
update interfaces when the second actual import happens, so the
`SocketInterface` class that gets instantiated holds a reference to the
old `socket_devices` dict. Removing the caching removes that problem as
well. If there's no caching, both the `SocketInterface` class and
`IngenicoDriver` will hold a reference to the same instance of
`socket_devices`.
parent 6c24d30c
Branches
Tags
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment