Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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:

Section cache is properly invalidated by Configuration.parse_if_needed(..).

API 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)

T11902_config_parse_if_needed_invalidate_cache.patch (3.1 KB ) - added by Peter Suter 5 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by Ryan J Ollos, 5 years ago

Cc: Ryan J Ollos added

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.

in reply to:  1 comment:2 by Peter Suter, 5 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.

comment:3 by Ryan J Ollos, 5 years ago

I added those changes to rjollos.git:t11902-parent-invalidates-config-cache.

in reply to:  3 comment:4 by Jun Omae, 5 years ago

I added those changes to rjollos.git:t11902-parent-invalidates-config-cache.

Looks good to me.

comment:5 by Ryan J Ollos, 5 years ago

Milestone: 1.0.3
Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Committed to 1.0-stable in [13628], merged to trunk in [13629].

comment:6 by Ryan J Ollos, 5 years ago

Owner: set to Peter Suter

comment:7 by Ryan J Ollos, 5 years ago

Test failures on Windows reported in #12019.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Peter Suter.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Peter Suter to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.