honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Watcher's ExecutionService is not thread-safe. (elastic/x-pack-elasticsearch#3861) 

If start, stop and/or clearExecutions are called concurrently, you could end up
in an inconsistent state where `currentExecutions.sealAndAwaitEmpty` is called
twice on the same instance.

These `synchronized` keywords should make it a bit better, though I suspect
there are other issues and thread safety of this class should be reviewed.

This might have caused
https://internal-ci.elastic.co/job/elastic+x-pack-elasticsearch+5.6+multijob-unix-compatibility/932/consoleFull

Original commit: elastic/x-pack-elasticsearch@2f7e7a34d9
This commit is contained in:
Adrien Grand 2018-02-13 11:27:11 +01:00 committed by GitHub
parent 19e9afdf10
commit c633b919ab
1 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java
View File

@ -109,7 +109,7 @@ public class ExecutionService extends AbstractComponent {
this.indexDefaultTimeout = settings.getAsTime("xpack.watcher.internal.ops.index.default_timeout", TimeValue.timeValueSeconds(30));
}
public void start() throws Exception {
public synchronized void start() throws Exception {
if (started.get()) {
return;
}
@ -133,7 +133,7 @@ public class ExecutionService extends AbstractComponent {
return triggeredWatchStore.validate(state) && HistoryStore.validate(state);
}
public void stop() {
public synchronized void stop() {
if (started.compareAndSet(true, false)) {
logger.debug("stopping execution service");
// We could also rely on the shutdown in #updateSettings call, but
@ -541,7 +541,7 @@ public class ExecutionService extends AbstractComponent {
* This clears out the current executions and sets new empty current executions
* This is needed, because when this method is called, watcher keeps running, so sealing executions would be a bad idea
*/
public void clearExecutions() {
public synchronized void clearExecutions() {
currentExecutions.sealAndAwaitEmpty(maxStopTimeout);
currentExecutions = new CurrentExecutions();
}