[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: Text File reloading_patch.txt    
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.
For this scenario it would be best that reloading happens only once, but not with the first event, but with the last one.

idea:

  • define queueing counter (init to 0)
  • method reload():
      • increase counter
      • sleep 1 sec
      • synchronize {
          • decrease counter
          • if (counter == 0) { run reloading code }
          • }
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.
With the first reload request the a Reloader will be started in a separate thread. The Reloader checks if the number of reload requests increases. If true it will sleep for a second, if false it will doe the reloading tasks.

I attached the patch for ObservedManager. If you have difficulties applying the path please call

Comment by Philipp Bracher [ 19/Apr/06 ]

Commited thanks!

Generated at Mon Feb 12 03:20:55 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.