#39 closed enhancement (fixed)
Make Template.generate() reentrant
Reported by: | cboos | Owned by: | cmlenz |
---|---|---|---|
Priority: | critical | Milestone: | 0.3 |
Component: | Template processing | Version: | 0.2 |
Keywords: | concurrency threadsafe | Cc: |
Description
As I understand it, it is currently not possible to reuse a template from different threads. For example, Trac would require this if we were to store a TemplateLoader in the environment.
This is because there are several directives which currently maintain an internal state, during __call__():
- the choose/when/otherwise logic is done by checking and setting the self.matched and self.value attributes of the ChooseDirective.
However, that ChooseDirective is itself stored in the context (choose = ctxt['_choose']). Instead, the matched and value should be stored directly in the context (ctxt['_choose_matched'] and ctxt['_choose_value'] for example). - the MatchDirective stores a copy of the stream in self.stream but also appends it to the ctxt._match_templates. Isn't the latter operation enough, as I don't see where that self.stream is being used?
If that is all there is to it, then I could write a patch myself... But I prefer to ask first if there's is anything more complex which I overlooked!
Attachments (2)
Change History (5)
comment:1 Changed 18 years ago by cmlenz
Changed 18 years ago by cboos
Cache the template loader in Trac -- makes it easy to stress test reentrancy issues.
comment:2 Changed 18 years ago by cboos
OK, so it appears that's all there is to it ;)
In Trac's Markup branch, with attachment:env_load_template-r3652.patch applied, all seems well for over 30 simultaneous requests on the same complex template (ticket_view.html), when using the proposed implementation (attachment:template_generate_now_reentrant-r250.patch).
Not to mention it's way faster, as that was to be expected ;)
comment:3 Changed 18 years ago by cmlenz
- Resolution set to fixed
- Status changed from new to closed
Applied slightly modified patch in [252]. Thanks!
Very good point. These things should all be done through the context as you propose. I don't think there's anything you've overlooked, but I'm not 100% sure.