model: prevent simultan access of singleton (#773)

The singleton instance of "Diagnostician" is used without
synchronization.
The singleton "Diasnostician" instance is using the singleton
"EValidator.Registry" instance (without synchronization).
In very rare high load situations there has been CME detected.

My first "solution" has been to synchronize the access of
Diagnostician's instance method by using

```java
synchronized (Diagnostician.INSTANCE) {
    ...
```

But after realize that EMF is using internally other singletons I tried
to find any information about EMF and thread safety.

I found this one: https://javahacks.net/2016/07/13/emf-thread-safety/

  EMF models are not thread-safe by default and writing multithreaded
  applications is not that simple.
  The more complex our application became, the more often we got
  concurrent modification exceptions and had problems with filtering and
  sorting operations.

So, I assume instead of adding synchronizations to our code that is
using EMF (IIRC this has been already done on the ESH hosted code base
long time ago) we should try to execute EMF code in a safe manner.

This implementation adds a "SafeEMF" OSGi service that should be used to
execute EMF code to ensure none code of EMF (or at least the ones that
calls has been migrated to the SafeEMF usage) is accessed by separate
threads at the same time.

Related to: https://github.com/openhab/openhab-core/issues/772

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>
pull/781/head
Markus Rathgeb 2019-05-01 23:19:07 +02:00 committed by Kai Kreuzer
parent bafc39f91b
commit f881ef87f1
3 changed files with 89 additions and 3 deletions

View File

@ -0,0 +1,40 @@
/**
* Copyright (c) 2010-2019 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.smarthome.model.core;
import java.util.function.Supplier;
/**
* Service interface to execute EMF methods in a single based thread.
*
* @author Markus Rathgeb - Initial contribution
*/
public interface SafeEMF {
/**
* Calls the given function.
*
* @param <T> the return type of the calling function
* @param func the function to call
* @return the return value of the called function
*/
<T> T call(Supplier<T> func);
/**
* Calls the given function.
*
* @param func the function to call
*/
void call(Runnable func);
}

View File

@ -38,10 +38,13 @@ import org.eclipse.emf.ecore.util.Diagnostician;
import org.eclipse.smarthome.model.core.EventType;
import org.eclipse.smarthome.model.core.ModelRepository;
import org.eclipse.smarthome.model.core.ModelRepositoryChangeListener;
import org.eclipse.smarthome.model.core.SafeEMF;
import org.eclipse.xtext.resource.SynchronizedXtextResourceSet;
import org.eclipse.xtext.resource.XtextResource;
import org.eclipse.xtext.resource.XtextResourceSet;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -58,7 +61,12 @@ public class ModelRepositoryImpl implements ModelRepository {
private final List<ModelRepositoryChangeListener> listeners = new CopyOnWriteArrayList<>();
public ModelRepositoryImpl() {
private final SafeEMF safeEmf;
@Activate
public ModelRepositoryImpl(final @Reference SafeEMF safeEmf) {
this.safeEmf = safeEmf;
XtextResourceSet xtextResourceSet = new SynchronizedXtextResourceSet();
xtextResourceSet.addLoadOption(XtextResource.OPTION_RESOLVE_ALL, Boolean.TRUE);
this.resourceSet = xtextResourceSet;
@ -278,8 +286,8 @@ public class ModelRepositoryImpl implements ModelRepository {
// Check for validation errors, but log them only
try {
org.eclipse.emf.common.util.Diagnostic diagnostic = Diagnostician.INSTANCE
.validate(resource.getContents().get(0));
final org.eclipse.emf.common.util.Diagnostic diagnostic = safeEmf
.call(() -> Diagnostician.INSTANCE.validate(resource.getContents().get(0)));
for (org.eclipse.emf.common.util.Diagnostic d : diagnostic.getChildren()) {
warnings.add(d.getMessage());
}

View File

@ -0,0 +1,38 @@
/**
* Copyright (c) 2010-2019 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.smarthome.model.core.internal;
import java.util.function.Supplier;
import org.eclipse.smarthome.model.core.SafeEMF;
import org.osgi.service.component.annotations.Component;
/**
* Implementation of a safe EMF caller..
*
* @author Markus Rathgeb - Initial contribution
*/
@Component
public class SafeEMFImpl implements SafeEMF {
@Override
public synchronized <T> T call(Supplier<T> func) {
return func.get();
}
@Override
public synchronized void call(Runnable func) {
func.run();
}
}