Merge lp:~mpontillo/maas/interface-up-down-visited-recursion-prevent into lp:maas/trunk

Proposed by Mike Pontillo on 2016-09-10
Status: Rejected
Rejected by: MAAS Lander on 2017-06-22
Proposed branch: lp:~mpontillo/maas/interface-up-down-visited-recursion-prevent
Merge into: lp:maas/trunk
Diff against target: 63 lines (+26/-12)
1 file modified
src/maasserver/models/signals/interfaces.py (+26/-12)
To merge this branch: bzr merge lp:~mpontillo/maas/interface-up-down-visited-recursion-prevent
Reviewer Review Type Date Requested Status
MAAS Maintainers 2016-09-10 Pending
Review via email: mp+305403@code.launchpad.net

Commit message

Add visited pattern to interface_enabled_or_disabled post-save signal handler for Interface model objects, to fix an infinite recursion bug seen during end-to-end testing while interfaces were being enabled and disabled on a rack controller.

To post a comment you must log in.
MAAS Lander (maas-lander) wrote :

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5342. By Mike Pontillo on 2016-09-10

Fix bug that caused infinite recursion in some cases when an interface was visited twice by the post-save signal after toggling between disabled and enabled.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/signals/interfaces.py'
2--- src/maasserver/models/signals/interfaces.py 2016-07-30 01:17:54 +0000
3+++ src/maasserver/models/signals/interfaces.py 2016-09-10 07:09:35 +0000
4@@ -45,6 +45,28 @@
5 signals = SignalsManager()
6
7
8+class InterfaceVisitingThreadLocal(threading.local):
9+ """Since infinite recursion could occur in an arbitrary interface
10+ hierarchy, use thread-local storage to ensure that each interface is only
11+ visited once.
12+ """
13+ def __init__(self):
14+ super().__init__()
15+ self.visiting = set()
16+
17+enabled_or_disabled_thread_local = InterfaceVisitingThreadLocal()
18+
19+
20+def ensure_link_up(interface):
21+ visiting = enabled_or_disabled_thread_local.visiting
22+ if interface.id not in visiting:
23+ try:
24+ visiting.add(interface.id)
25+ interface.ensure_link_up()
26+ finally:
27+ visiting.discard(interface.id)
28+
29+
30 def interface_enabled_or_disabled(instance, old_values, **kwargs):
31 """When an interface is enabled be sure at minimum a LINK_UP is created.
32 When an interface is disabled make sure that all its links are removed,
33@@ -53,9 +75,10 @@
34 return
35 if instance.is_enabled():
36 # Make sure it has a LINK_UP link, and for its children.
37- instance.ensure_link_up()
38+ ensure_link_up(instance)
39 for rel in instance.children_relationships.all():
40- rel.child.ensure_link_up()
41+ ensure_link_up(rel.child)
42+
43 else:
44 # Was disabled. Remove the links.
45 for ip_address in instance.ip_addresses.exclude(
46@@ -140,16 +163,7 @@
47 klass, ['params'], delete=False)
48
49
50-class InterfaceUpdateParentsThreadLocal(threading.local):
51- """Since infinite recursion could occur in an arbitrary interface
52- hierarchy, use thread-local stroage to ensure that each interface is only
53- visited once.
54- """
55- def __init__(self):
56- super().__init__()
57- self.visiting = set()
58-
59-update_parents_thread_local = InterfaceUpdateParentsThreadLocal()
60+update_parents_thread_local = InterfaceVisitingThreadLocal()
61
62
63 def update_interface_parents(sender, instance, created, **kwargs):