diff options
author | 2021-10-31 16:56:34 +0100 | |
---|---|---|
committer | 2021-10-31 19:26:16 +0100 | |
commit | 4808ac1571adb95fa2ffc960c95b4b46f5d5fbe2 (patch) | |
tree | d45afe83b29fd15294c73d160b72644ee5c8afef /language-web/src/main/java/tools | |
parent | chore(web): refactor xtext client (diff) | |
download | refinery-4808ac1571adb95fa2ffc960c95b4b46f5d5fbe2.tar.gz refinery-4808ac1571adb95fa2ffc960c95b4b46f5d5fbe2.tar.zst refinery-4808ac1571adb95fa2ffc960c95b4b46f5d5fbe2.zip |
fix(web): fix push message race condition
In some resource-constrained environments (e.g., my VPS with 2 vCPUs),
it was possible for validation and highlighting precomputation to be
finished before the server responded to a deltaText updating completion
request (updating completion take longer than a normal update, because
they also have to compute the completions). Therefore, the client
received push messages about a stateId it didn't know about yet.
To fix this, we delay any push messages originating during servicing a
call to be sent after the call is serviced. Thus the client first
receives the updating completion response with the new stateId, followed
by the push messages relating to that stateId.
Diffstat (limited to 'language-web/src/main/java/tools')
-rw-r--r-- | language-web/src/main/java/tools/refinery/language/web/xtext/server/TransactionExecutor.java | 46 |
1 files changed, 45 insertions, 1 deletions
diff --git a/language-web/src/main/java/tools/refinery/language/web/xtext/server/TransactionExecutor.java b/language-web/src/main/java/tools/refinery/language/web/xtext/server/TransactionExecutor.java index 335f0636..0b417b06 100644 --- a/language-web/src/main/java/tools/refinery/language/web/xtext/server/TransactionExecutor.java +++ b/language-web/src/main/java/tools/refinery/language/web/xtext/server/TransactionExecutor.java | |||
@@ -1,7 +1,9 @@ | |||
1 | package tools.refinery.language.web.xtext.server; | 1 | package tools.refinery.language.web.xtext.server; |
2 | 2 | ||
3 | import java.lang.ref.WeakReference; | 3 | import java.lang.ref.WeakReference; |
4 | import java.util.ArrayList; | ||
4 | import java.util.HashMap; | 5 | import java.util.HashMap; |
6 | import java.util.List; | ||
5 | import java.util.Map; | 7 | import java.util.Map; |
6 | 8 | ||
7 | import org.eclipse.emf.common.util.URI; | 9 | import org.eclipse.emf.common.util.URI; |
@@ -13,6 +15,8 @@ import org.eclipse.xtext.web.server.ISession; | |||
13 | import org.eclipse.xtext.web.server.InvalidRequestException; | 15 | import org.eclipse.xtext.web.server.InvalidRequestException; |
14 | import org.eclipse.xtext.web.server.InvalidRequestException.UnknownLanguageException; | 16 | import org.eclipse.xtext.web.server.InvalidRequestException.UnknownLanguageException; |
15 | import org.eclipse.xtext.web.server.XtextServiceDispatcher; | 17 | import org.eclipse.xtext.web.server.XtextServiceDispatcher; |
18 | import org.slf4j.Logger; | ||
19 | import org.slf4j.LoggerFactory; | ||
16 | 20 | ||
17 | import com.google.common.base.Strings; | 21 | import com.google.common.base.Strings; |
18 | import com.google.inject.Injector; | 22 | import com.google.inject.Injector; |
@@ -27,6 +31,8 @@ import tools.refinery.language.web.xtext.server.push.PushWebDocument; | |||
27 | import tools.refinery.language.web.xtext.servlet.SimpleServiceContext; | 31 | import tools.refinery.language.web.xtext.servlet.SimpleServiceContext; |
28 | 32 | ||
29 | public class TransactionExecutor implements IDisposable, PrecomputationListener { | 33 | public class TransactionExecutor implements IDisposable, PrecomputationListener { |
34 | private static final Logger LOG = LoggerFactory.getLogger(TransactionExecutor.class); | ||
35 | |||
30 | private final ISession session; | 36 | private final ISession session; |
31 | 37 | ||
32 | private final IResourceServiceProvider.Registry resourceServiceProviderRegistry; | 38 | private final IResourceServiceProvider.Registry resourceServiceProviderRegistry; |
@@ -35,6 +41,12 @@ public class TransactionExecutor implements IDisposable, PrecomputationListener | |||
35 | 41 | ||
36 | private ResponseHandler responseHandler; | 42 | private ResponseHandler responseHandler; |
37 | 43 | ||
44 | private Object callPendingLock = new Object(); | ||
45 | |||
46 | private boolean callPending; | ||
47 | |||
48 | private List<XtextWebPushMessage> pendingPushMessages = new ArrayList<>(); | ||
49 | |||
38 | public TransactionExecutor(ISession session, IResourceServiceProvider.Registry resourceServiceProviderRegistry) { | 50 | public TransactionExecutor(ISession session, IResourceServiceProvider.Registry resourceServiceProviderRegistry) { |
39 | this.session = session; | 51 | this.session = session; |
40 | this.resourceServiceProviderRegistry = resourceServiceProviderRegistry; | 52 | this.resourceServiceProviderRegistry = resourceServiceProviderRegistry; |
@@ -51,6 +63,15 @@ public class TransactionExecutor implements IDisposable, PrecomputationListener | |||
51 | responseHandler.onResponse(new XtextWebOkResponse(request, new PongResult(ping))); | 63 | responseHandler.onResponse(new XtextWebOkResponse(request, new PongResult(ping))); |
52 | return; | 64 | return; |
53 | } | 65 | } |
66 | synchronized (callPendingLock) { | ||
67 | if (callPending) { | ||
68 | LOG.error("Reentrant request detected"); | ||
69 | } | ||
70 | if (!pendingPushMessages.isEmpty()) { | ||
71 | LOG.error("{} push messages got stuck without a pending request", pendingPushMessages.size()); | ||
72 | } | ||
73 | callPending = true; | ||
74 | } | ||
54 | try { | 75 | try { |
55 | var injector = getInjector(serviceContext); | 76 | var injector = getInjector(serviceContext); |
56 | var serviceDispatcher = injector.getInstance(XtextServiceDispatcher.class); | 77 | var serviceDispatcher = injector.getInstance(XtextServiceDispatcher.class); |
@@ -61,13 +82,36 @@ public class TransactionExecutor implements IDisposable, PrecomputationListener | |||
61 | responseHandler.onResponse(new XtextWebErrorResponse(request, XtextWebErrorKind.REQUEST_ERROR, e)); | 82 | responseHandler.onResponse(new XtextWebErrorResponse(request, XtextWebErrorKind.REQUEST_ERROR, e)); |
62 | } catch (RuntimeException e) { | 83 | } catch (RuntimeException e) { |
63 | responseHandler.onResponse(new XtextWebErrorResponse(request, XtextWebErrorKind.SERVER_ERROR, e)); | 84 | responseHandler.onResponse(new XtextWebErrorResponse(request, XtextWebErrorKind.SERVER_ERROR, e)); |
85 | } finally { | ||
86 | synchronized (callPendingLock) { | ||
87 | for (var message : pendingPushMessages) { | ||
88 | try { | ||
89 | responseHandler.onResponse(message); | ||
90 | } catch (ResponseHandlerException | RuntimeException e) { | ||
91 | LOG.error("Error while flushing push message", e); | ||
92 | } | ||
93 | } | ||
94 | pendingPushMessages.clear(); | ||
95 | callPending = false; | ||
96 | } | ||
64 | } | 97 | } |
65 | } | 98 | } |
66 | 99 | ||
67 | @Override | 100 | @Override |
68 | public void onPrecomputedServiceResult(String resourceId, String stateId, String serviceName, | 101 | public void onPrecomputedServiceResult(String resourceId, String stateId, String serviceName, |
69 | IServiceResult serviceResult) throws ResponseHandlerException { | 102 | IServiceResult serviceResult) throws ResponseHandlerException { |
70 | responseHandler.onResponse(new XtextWebPushMessage(resourceId, stateId, serviceName, serviceResult)); | 103 | var message = new XtextWebPushMessage(resourceId, stateId, serviceName, serviceResult); |
104 | synchronized (callPendingLock) { | ||
105 | // If we're currently responding to a call we must delay any push messages until | ||
106 | // the reply is sent, because push messages relating to the new state id must be | ||
107 | // sent after the response with the new state id so that the client knows about | ||
108 | // the new state when it receives the push message. | ||
109 | if (callPending) { | ||
110 | pendingPushMessages.add(message); | ||
111 | } else { | ||
112 | responseHandler.onResponse(message); | ||
113 | } | ||
114 | } | ||
71 | } | 115 | } |
72 | 116 | ||
73 | @Override | 117 | @Override |