commit 87b747a7c8c1a3a9ae3d2b45778b02bde0463163 Author: Terry Wilson Date: Wed Sep 23 18:46:53 2020 +0000 Avoid race condition with RowEvent handling Row objects are inherently tied to the transaction processing of the Idl. This means that if you have a reference to a Row in one thread, and another thread starts a transaction that modifies that row, the Row can change w/o you knowing it. This is especially noticeable when using the RowEventHandler. It is possible for a Row that is passed to notify() by the Idl class to change between being matched and the RowEvent.run() method being called. This patch returns an immutable representation of the row by using the same class that custom indexes use for searching. This should be safe to pass to other threads. Change-Id: Iff6e9fdfe032e1c007e35fc7b018114e66acc895 Closes-Bug: #1896816 (cherry picked from commit dc09d6696fe175e53c0c1e4a4263d1a3e1513c9c) diff --git a/ovsdbapp/backend/ovs_idl/event.py b/ovsdbapp/backend/ovs_idl/event.py index 4fbe960..33491d1 100644 --- a/ovsdbapp/backend/ovs_idl/event.py +++ b/ovsdbapp/backend/ovs_idl/event.py @@ -47,3 +47,9 @@ class RowEvent(ovsdb_event.RowEvent): # pylint: disable=abstract-method class WaitEvent(RowEvent, ovsdb_event.WaitEvent): pass + + +class RowEventHandler(ovsdb_event.RowEventHandler): + def notify(self, event, row, updates=None): + row = idlutils.frozen_row(row) + super(RowEventHandler, self).notify(event, row, updates) diff --git a/ovsdbapp/backend/ovs_idl/idlutils.py b/ovsdbapp/backend/ovs_idl/idlutils.py index ec53f57..d0e730b 100644 --- a/ovsdbapp/backend/ovs_idl/idlutils.py +++ b/ovsdbapp/backend/ovs_idl/idlutils.py @@ -428,3 +428,22 @@ def row2str(row): return "%s(%s)" % (row._table.name, ", ".join( "%s=%s" % (col, idl._row_to_uuid(getattr(row, col))) for col in row._table.columns if hasattr(row, col))) + + +def frozen_row(row): + """Return a namedtuple representation of a idl.Row object + + Row objects are inherently tied to the transaction processing of the Idl. + This means that if you have a reference to a Row in one thread, and + another thread starts a transaction that modifies that row, the Row can + change w/o you knowing it. This is especially noticeable when using the + RowEventHandler. It is possible for a Row that is passed to notify() by + the Idl class to change between being matched and the RowEvent.run() + method being called. This returns an immutable representation of the row + by using the same class that custom indexes use for searching. This + should be safe to pass to other threads. + """ + return row._table.rows.IndexEntry( + uuid=row.uuid, + **{col: getattr(row, col) + for col in row._table.columns if hasattr(row, col)}) diff --git a/ovsdbapp/event.py b/ovsdbapp/event.py index f2902f0..4a0f92f 100644 --- a/ovsdbapp/event.py +++ b/ovsdbapp/event.py @@ -166,6 +166,17 @@ class RowEventHandler(object): LOG.exception('Unexpected exception in notify_loop') def notify(self, event, row, updates=None): + """Method for calling backend to call for each DB update + + :param event: Backend representation of event type, e.g. + create, update, delete + :param row: Backend representation of a Row object. If it is not + immutable, it should be converted or guaranteed not to + be changed in other threads. + :param updates: Backend representation of updates to a Row. e.g. + a Row object with just changed attributes, a + dictionary of changes, etc. + """ matching = self.matching_events( event, row, updates) for match in matching: diff --git a/ovsdbapp/tests/functional/schema/ovn_southbound/test_impl_idl.py b/ovsdbapp/tests/functional/schema/ovn_southbound/test_impl_idl.py index 082b154..b173673 100644 --- a/ovsdbapp/tests/functional/schema/ovn_southbound/test_impl_idl.py +++ b/ovsdbapp/tests/functional/schema/ovn_southbound/test_impl_idl.py @@ -10,8 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +from ovsdbapp.backend.ovs_idl import event as ovsdb_event from ovsdbapp.backend.ovs_idl import idlutils -from ovsdbapp import event as ovsdb_event from ovsdbapp.schema.ovn_northbound import impl_idl as nbidl from ovsdbapp.schema.ovn_southbound import impl_idl from ovsdbapp.tests.functional import base