|
This is indeed a classic deadlock scenario. I can reproduce the problem with a unit test.
While testing I found another problem related to reloading strategies. I created A fix was applied to trunk in revision 712401.
So far, accessing the root node of the combined configuration was synchronized because it had to be re-constructed if something had changed. This fix removes this synchronization and - to ensure proper visibility - adds the volatile modifier to the root node field. If now multiple threads want to access the combined configuration, there may be a race condition causing the root node to be constructed multiple times. However, this does not impact the results of queries or have other undesired side effects. When testing I found out that reloading operations are indeed problematic when multiple threads are involved. The unit test case uses a synthetic reloading strategy that forces a reload on every access. AbstractFileConfiguration overrides all access methods to ensure that reload() is invoked as in the following example: public Object getProperty(String key) { reload(); return super.getProperty(key); } If there are multiple concurrent reloads or property accesses in sequence - as in the test I created -, the following race condition can happen:
To get my test running I changed the implementation of getProperty() in the following way: public Object getProperty(String key) { synchronized (reloadLock) { reload(); return super.getProperty(key); } } So the whole property read operation is now guarded by the lock. To be on the safe side this change would have to be applied for the other methods, too. But I think, the architecture of the reloading implementation should better be re-worked in the next major release. Note that it is very unlikely that multiple reloads happen in short intervals because the available reloading strategies have a refresh delay. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Critical
Deadlock during refresh properties
Because getNodeCombiner() is public method i can grab lock in my code first before try to do reloading.
synchronized (combinedConfiguration.getNodeCombiner()){
//do the reloading
}
It means locks are grabbed always in the same order so deadlock can't happen.
Best
Pavel