#11902 closed defect (fixed)
Fix config.parse_if_needed cache invalidation
Reported by: | Peter Suter | Owned by: | Peter Suter |
---|---|---|---|
Priority: | normal | Milestone: | 1.0.3 |
Component: | general | Version: | |
Severity: | normal | Keywords: | |
Cc: | Ryan J Ollos | Branch: | |
Release Notes: |
|
||
API Changes: | |||
Internal Changes: |
Description
42:ticket:5525 noted:
It appears there is an unused protected attribute
_cache
in [9037#file11]: tags/trac-1.0.2/trac/config.py@:293-294#L264. Could that be removed?
But perhaps some other variable was meant to be cleared, e.g. self._sections = {}
or for k in self._sections: self._sections[k]._cache = {}
.
It looks like this scenario would currently fail and be fixed by that cache invalidation:
# config inherits parent.ini containing [a] b=x config.get('a', 'b') # caches 'x' in config._sections['a']._cache['b'] # change parent.ini to [a] b=y config.parse_if_needed() # does not invalidate that cache config.get('a', 'b') # should return 'y' but returns cached 'x'
We should add a testcase and fix the code.
Attachments (1)
Change History (8)
by , 10 years ago
Attachment: | T11902_config_parse_if_needed_invalidate_cache.patch added |
---|
follow-up: 2 comment:1 by , 10 years ago
Cc: | added |
---|
comment:2 by , 10 years ago
Replying to rjollos:
I was having some trouble reproducing any issues in a Trac instance. It looks like this is due to the Environment object being destroyed when a configuration change is detected.
Yes, sorry I forgot to mention that. With Trac alone as a blackbox the issue does not seem to be externally observable. The calls to parse_if_needed
in Trac seem to be uncritical due to circumstances as pointed out.
I did not thoroughly check plugins, but it looks like th:AccountManagerPlugin may be affected.
follow-up: 4 comment:3 by , 10 years ago
- It looks like an instance of
self._sections = {}
can be removed: tags/trac-1.0.2/trac/config.py@:271#L264. The mainConfiguration
object and every parent will setchanged = True
, which will clear_sections
before the recursively-called function exits. - Maybe we could use wait_for_file_mtime_change to avoid the
time.sleep(2)
?
I added those changes to rjollos.git:t11902-parent-invalidates-config-cache.
comment:4 by , 10 years ago
…
I added those changes to rjollos.git:t11902-parent-invalidates-config-cache.
Looks good to me.
comment:5 by , 10 years ago
Milestone: | → 1.0.3 |
---|---|
Release Notes: | modified (diff) |
Resolution: | → fixed |
Status: | new → closed |
comment:6 by , 10 years ago
Owner: | set to |
---|
The patch looks correct, but I was having some trouble reproducing any issues in a Trac instance. It looks like this is due to the Environment object being destroyed when a configuration change is detected. Regardless, I think we should fix the issue. I have a minor refinement that I'll post shortly.