Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #15849 -- Made IfChanged node thread safe.

Previously, the ifchanged node stored state on `self._last_seen`,
thereby giving undesired results when the node is reused by another
thread at the same time (e.g. globally caching a Template object).

Thanks to akaihola for the report and Diederik van der Boor and
Bas Peschier for the patch.
  • Loading branch information...
commit 8503120c1024ad7ec2151196a743d6daec21334b 1 parent a5733fc
@vdboor vdboor authored apollo13 committed
View
30 django/template/defaulttags.py
@@ -215,32 +215,44 @@ class IfChangedNode(Node):
def __init__(self, nodelist_true, nodelist_false, *varlist):
self.nodelist_true, self.nodelist_false = nodelist_true, nodelist_false
- self._last_seen = None
self._varlist = varlist
- self._id = str(id(self))
def render(self, context):
- if 'forloop' in context and self._id not in context['forloop']:
- self._last_seen = None
- context['forloop'][self._id] = 1
+ # Init state storage
+ state_frame = self._get_context_stack_frame(context)
+ if self not in state_frame:
+ state_frame[self] = None
+
try:
if self._varlist:
# Consider multiple parameters. This automatically behaves
# like an OR evaluation of the multiple variables.
compare_to = [var.resolve(context, True) for var in self._varlist]
else:
+ # The "{% ifchanged %}" syntax (without any variables) compares the rendered output.
compare_to = self.nodelist_true.render(context)
except VariableDoesNotExist:
compare_to = None
- if compare_to != self._last_seen:
- self._last_seen = compare_to
- content = self.nodelist_true.render(context)
- return content
+ if compare_to != state_frame[self]:
+ state_frame[self] = compare_to
+ return self.nodelist_true.render(context)
elif self.nodelist_false:
return self.nodelist_false.render(context)
return ''
+ def _get_context_stack_frame(self, context):
+ # The Context object behaves like a stack where each template tag can create a new scope.
+ # Find the place where to store the state to detect changes.
+ if 'forloop' in context:
+ # Ifchanged is bound to the local for loop.
+ # When there is a loop-in-loop, the state is bound to the inner loop,
+ # so it resets when the outer loop continues.
+ return context['forloop']
+ else:
+ # Using ifchanged outside loops. Effectively this is a no-op because the state is associated with 'self'.
+ return context.render_context
+
class IfEqualNode(Node):
child_nodelists = ('nodelist_true', 'nodelist_false')
View
21 tests/regressiontests/templates/tests.py
@@ -420,6 +420,27 @@ def test_invalid_block_suggestion(self):
except TemplateSyntaxError as e:
self.assertEqual(e.args[0], "Invalid block tag: 'endblock', expected 'elif', 'else' or 'endif'")
+ def test_ifchanged_concurrency(self):
+ # Tests for #15849
+ template = Template('[0{% for x in foo %},{% with var=get_value %}{% ifchanged %}{{ var }}{% endifchanged %}{% endwith %}{% endfor %}]')
+
+ # Using generator to mimic concurrency.
+ # The generator is not passed to the 'for' loop, because it does a list(values)
+ # instead, call gen.next() in the template to control the generator.
+ def gen():
+ yield 1
+ yield 2
+ # Simulate that another thread is now rendering.
+ # When the IfChangeNode stores state at 'self' it stays at '3' and skip the last yielded value below.
+ iter2 = iter([1, 2, 3])
+ output2 = template.render(Context({'foo': range(3), 'get_value': lambda: next(iter2)}))
+ self.assertEqual(output2, '[0,1,2,3]', 'Expected [0,1,2,3] in second parallel template, got {0}'.format(output2))
+ yield 3
+
+ gen1 = gen()
+ output1 = template.render(Context({'foo': range(3), 'get_value': lambda: next(gen1)}))
+ self.assertEqual(output1, '[0,1,2,3]', 'Expected [0,1,2,3] in first template, got {0}'.format(output1))
+
def test_templates(self):
template_tests = self.get_template_tests()
filter_tests = filters.get_filter_tests()
Please sign in to comment.
Something went wrong with that request. Please try again.