Skip to content

Commit b290371

Browse files
mkjsixmkjsix
committed
fix: ensure context operations in JSInterceptorFactory and JSStringService run on platform thread to prevent thread-local issues
See #663
1 parent 97512a8 commit b290371

2 files changed

Lines changed: 102 additions & 89 deletions

File tree

‎polyglot/src/main/java/org/restheart/polyglot/interceptors/JSInterceptorFactory.java‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,14 @@ public JSInterceptorFactory(Optional<MongoClient> mclient, Configuration config)
107107
var sindexPath = pluginPath.toUri().toString();
108108
LOGGER.debug("Resolved interceptor path: {}", sindexPath);
109109

110+
// Context creation, eval and all Value member access must happen on the very
111+
// same platform thread: a Value returned by eval() is only safely readable on
112+
// the thread the context is entered on (see PolyglotThreadUtils), so the whole
113+
// block below -- not just the eval() calls -- is dispatched as a single task.
114+
try {
115+
return PolyglotThreadUtils.onPlatformThread(() -> {
110116
var ctx = ContextQueue.newContext(engine, "foo", config, LOGGER, mclient, "", contextOptions);
117+
ctx.enter();
111118
try {
112119

113120
// ******** evaluate and check options
@@ -118,8 +125,7 @@ public JSInterceptorFactory(Optional<MongoClient> mclient, Configuration config)
118125
Value options;
119126

120127
try {
121-
// ctx.eval() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
122-
options = PolyglotThreadUtils.onPlatformThread(() -> ctx.eval(optionsSource));
128+
options = ctx.eval(optionsSource);
123129
} catch (Throwable t) {
124130
throw new IllegalArgumentException("wrong js interceptor, " + t.getMessage());
125131
}
@@ -209,8 +215,7 @@ public JSInterceptorFactory(Optional<MongoClient> mclient, Configuration config)
209215
Value handle;
210216

211217
try {
212-
// ctx.eval() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
213-
handle = PolyglotThreadUtils.onPlatformThread(() -> ctx.eval(handleSource));
218+
handle = ctx.eval(handleSource);
214219
} catch (Throwable t) {
215220
throw new IllegalArgumentException(
216221
"wrong js interceptor " + pluginPath.toAbsolutePath() + ", " + t.getMessage());
@@ -229,8 +234,7 @@ public JSInterceptorFactory(Optional<MongoClient> mclient, Configuration config)
229234
Value resolve;
230235

231236
try {
232-
// ctx.eval() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
233-
resolve = PolyglotThreadUtils.onPlatformThread(() -> ctx.eval(resolveSource));
237+
resolve = ctx.eval(resolveSource);
234238
} catch (Throwable t) {
235239
throw new IllegalArgumentException(
236240
"wrong js interceptor " + pluginPath.toAbsolutePath() + ", " + t.getMessage());
@@ -353,15 +357,18 @@ public JSInterceptorFactory(Optional<MongoClient> mclient, Configuration config)
353357
interceptor,
354358
new HashMap<>());
355359
} finally {
356-
try {
357-
// Context.close() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
358-
PolyglotThreadUtils.onPlatformThread(() -> {
359-
ctx.close();
360-
return null;
361-
});
362-
} catch (Exception e) {
363-
LOGGER.warn("Error closing context for {}", pluginPath, e);
364-
}
360+
ctx.leave();
361+
ctx.close();
362+
}
363+
});
364+
} catch (RuntimeException re) {
365+
throw re;
366+
} catch (IOException ioe) {
367+
throw ioe;
368+
} catch (InterruptedException ie) {
369+
throw ie;
370+
} catch (Exception e) {
371+
throw new IllegalStateException("Error evaluating js interceptor " + pluginPath.toAbsolutePath(), e);
365372
}
366373
}
367374

‎polyglot/src/main/java/org/restheart/polyglot/services/JSStringService.java‎

Lines changed: 80 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -96,82 +96,88 @@ private static JSServiceArgs args(Path pluginPath, Optional<MongoClient> mclient
9696
LOGGER.trace("Enabling require for service {} with require-cwd {} ", pluginPath, requireCwdPath);
9797
}
9898

99-
var ctx = ContextQueue.newContext(engine(), "foo", config, LOGGER, mclient, "", contextOptions);
99+
// Context creation, eval and all Value member access must happen on the very
100+
// same platform thread: a Value returned by eval() is only safely readable on
101+
// the thread the context is entered on (see PolyglotThreadUtils), so the whole
102+
// block below -- not just the eval() calls -- is dispatched as a single task.
100103
try {
101-
// check that the plugin script is js (use PluginsClassloader so js-language is visible)
102-
final var language = PolyglotClassloaderHelper.withPluginsClassloaderResult(
103-
() -> Source.findLanguage(pluginPath.toFile()));
104-
105-
if (!"js".equals(language)) {
106-
throw new IllegalArgumentException("wrong js plugin, not javascript");
107-
}
108-
109-
var sindexPath = pluginPath.toUri().toString();
110-
LOGGER.debug("Resolved plugin path for import: {}", sindexPath);
111-
var optionsScript = "import { options } from '" + sindexPath + "'; options;";
112-
var optionsSource = Source.newBuilder(language, optionsScript, "optionsScript").mimeType("application/javascript+module").build();
113-
114-
Value options;
115-
116-
try {
117-
// ctx.eval() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
118-
options = PolyglotThreadUtils.onPlatformThread(() -> ctx.eval(optionsSource));
119-
} catch (Throwable t) {
120-
if (t.getMessage() != null && t.getMessage().contains("Cannot load CommonJS module")) {
121-
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage());
122-
} else if (t.getMessage() != null && t.getMessage().contains("Access to host class")) {
123-
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage());
124-
} else {
125-
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage() + ", " + PACKAGE_HINT);
126-
}
127-
}
128-
129-
checkOptions(options, pluginPath);
130-
131-
var name = options.getMember("name").asString();
132-
var description = options.getMember("description").asString();
133-
var uri = options.getMember("uri").asString();
134-
var secured = !options.getMemberKeys().contains("secured") ? false : options.getMember("secured").asBoolean();
135-
var matchPolicy = !options.getMemberKeys().contains("matchPolicy") ? MATCH_POLICY.PREFIX : MATCH_POLICY.valueOf(options.getMember("matchPolicy").asString());
136-
String modulesReplacements = null;
137-
138-
if (options.getMemberKeys().contains("modulesReplacements")) {
139-
var sb = new StringBuilder();
140-
141-
options.getMember("modulesReplacements").getMemberKeys().stream()
142-
.forEach(k -> sb.append(k).append(":")
143-
.append(options.getMember("modulesReplacements").getMember(k))
144-
.append(","));
145-
146-
modulesReplacements = sb.toString();
147-
}
148-
149-
// ******** evaluate and check handle
150-
var _handleScript = "import { handle } from '" + sindexPath + "'; handle;";
151-
var handleSource = Source.newBuilder(language, _handleScript, "handleScript").mimeType("application/javascript+module").build();
152-
153-
Value handle;
154-
155-
try {
156-
// ctx.eval() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
157-
handle = PolyglotThreadUtils.onPlatformThread(() -> ctx.eval(handleSource));
158-
} catch (Throwable t) {
159-
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ", " + t.getMessage());
160-
}
161-
162-
checkHandle(handle, pluginPath);
163-
164-
return new JSServiceArgs(name, description, uri, secured, modulesReplacements, matchPolicy, handleSource, config, mclient, contextOptions);
165-
} finally {
166-
try {
167-
// Context.close() touches thread locals, must run on a platform thread, see PolyglotThreadUtils
168-
PolyglotThreadUtils.onPlatformThread(() -> {
104+
return PolyglotThreadUtils.onPlatformThread(() -> {
105+
var ctx = ContextQueue.newContext(engine(), "foo", config, LOGGER, mclient, "", contextOptions);
106+
ctx.enter();
107+
try {
108+
// check that the plugin script is js (use PluginsClassloader so js-language is visible)
109+
final var language = PolyglotClassloaderHelper.withPluginsClassloaderResult(
110+
() -> Source.findLanguage(pluginPath.toFile()));
111+
112+
if (!"js".equals(language)) {
113+
throw new IllegalArgumentException("wrong js plugin, not javascript");
114+
}
115+
116+
var sindexPath = pluginPath.toUri().toString();
117+
LOGGER.debug("Resolved plugin path for import: {}", sindexPath);
118+
var optionsScript = "import { options } from '" + sindexPath + "'; options;";
119+
var optionsSource = Source.newBuilder(language, optionsScript, "optionsScript").mimeType("application/javascript+module").build();
120+
121+
Value options;
122+
123+
try {
124+
options = ctx.eval(optionsSource);
125+
} catch (Throwable t) {
126+
if (t.getMessage() != null && t.getMessage().contains("Cannot load CommonJS module")) {
127+
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage());
128+
} else if (t.getMessage() != null && t.getMessage().contains("Access to host class")) {
129+
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage());
130+
} else {
131+
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ": " + t.getMessage() + ", " + PACKAGE_HINT);
132+
}
133+
}
134+
135+
checkOptions(options, pluginPath);
136+
137+
var name = options.getMember("name").asString();
138+
var description = options.getMember("description").asString();
139+
var uri = options.getMember("uri").asString();
140+
var secured = !options.getMemberKeys().contains("secured") ? false : options.getMember("secured").asBoolean();
141+
var matchPolicy = !options.getMemberKeys().contains("matchPolicy") ? MATCH_POLICY.PREFIX : MATCH_POLICY.valueOf(options.getMember("matchPolicy").asString());
142+
String modulesReplacements = null;
143+
144+
if (options.getMemberKeys().contains("modulesReplacements")) {
145+
var sb = new StringBuilder();
146+
147+
options.getMember("modulesReplacements").getMemberKeys().stream()
148+
.forEach(k -> sb.append(k).append(":")
149+
.append(options.getMember("modulesReplacements").getMember(k))
150+
.append(","));
151+
152+
modulesReplacements = sb.toString();
153+
}
154+
155+
// ******** evaluate and check handle
156+
var _handleScript = "import { handle } from '" + sindexPath + "'; handle;";
157+
var handleSource = Source.newBuilder(language, _handleScript, "handleScript").mimeType("application/javascript+module").build();
158+
159+
Value handle;
160+
161+
try {
162+
handle = ctx.eval(handleSource);
163+
} catch (Throwable t) {
164+
throw new IllegalArgumentException("wrong js service " + pluginPath.toAbsolutePath() + ", " + t.getMessage());
165+
}
166+
167+
checkHandle(handle, pluginPath);
168+
169+
return new JSServiceArgs(name, description, uri, secured, modulesReplacements, matchPolicy, handleSource, config, mclient, contextOptions);
170+
} finally {
171+
ctx.leave();
169172
ctx.close();
170-
return null;
171-
});
172-
} catch (Exception e) {
173-
LOGGER.warn("Error closing context for {}", pluginPath, e);
174-
}
173+
}
174+
});
175+
} catch (RuntimeException re) {
176+
throw re;
177+
} catch (IOException ioe) {
178+
throw ioe;
179+
} catch (Exception e) {
180+
throw new IllegalStateException("Error evaluating js service " + pluginPath.toAbsolutePath(), e);
175181
}
176182
}
177183

0 commit comments

Comments
 (0)