[MAGNOLIA-815] Sync of ObservedManager.reload() does not work Created: 17/Apr/06 Updated: 23/Jan/13 Resolved: 19/Apr/06 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | 3.0 Beta 1 |
| Fix Version/s: | 3.0 Beta 1 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Ralf Hirning | Assignee: | Philipp Bärfuss |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | 1h | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | 1h | ||
| Attachments: |
|
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
| Date of First Response: |
| Description |
|
The Method ObservedManager.reload() is synchronized. Therefore whenever you enter this method this.reloading is false. So the mechanism to avoid multiple unnecessary reloads does not work. The check of this.reloading and the setting of this.reloading to false must not be synchronized. |
| Comments |
| Comment by Philipp Bracher [ 18/Apr/06 ] |
|
This is a check to avoid cyclic relaoding in the same thread --> synchronized is ignored. But I will double checkt the code again. |
| Comment by Philipp Bracher [ 18/Apr/06 ] |
|
I think the code is correct. Please reopen and comment if you see critical bahavior. Thanks |
| Comment by Ralf Hirning [ 18/Apr/06 ] |
|
The effect is that setting and checking of this.reloading will have no effect. When I install a templating package (about 200 entries in the config repository) it seems as if Magnolia runs into an endless loop. idea:
|
| Comment by Ralf Hirning [ 18/Apr/06 ] |
|
see last comment |
| Comment by Philipp Bracher [ 18/Apr/06 ] |
|
I like your approach, this saves a lot of performance during automatically changing of dialogs. We should definitely implement it. Can you send a patch or should I do it? About the scenario above, I don't understand why it enters an endless loop or a deadlock? Perhaps it starves due a a multiple reload triggered by multiple saves (out of memory ...). |
| Comment by Ralf Hirning [ 18/Apr/06 ] |
|
I will create a patch (but not today) Maybe I was not patient enough. After 10 mins I declared it an endless loop |
| Comment by Ralf Hirning [ 19/Apr/06 ] |
|
Just synchronizing did not work, so reloading will be done in a separate thread. I attached the patch for ObservedManager. If you have difficulties applying the path please call |
| Comment by Philipp Bracher [ 19/Apr/06 ] |
|
Commited thanks! |