Skip to content

Commit ec8642a

Browse files
Add site setting to configure server-side script execution timeout (#7862)
## Rationale The Rhino sandbox terminates any server-side JavaScript (such as trigger scripts) after a hard-coded 60 seconds of wall-clock time, which includes database and other Java operations invoked by the script. Long-running but legitimate operations, such as bulk imports that fire per-row triggers, can exceed this budget with no recourse. This change makes the timeout a configurable site setting so admins can raise it, or disable it entirely, without a code change. ## Related Pull Requests None. ## Changes - New `scriptExecutionTimeout` site setting (default 60 seconds, 0 disables) exposed on the Site Settings admin page and as a `SiteSettings.scriptExecutionTimeout` startup property. - The Rhino sandbox resolves the timeout once per script execution instead of using the hard-coded constant, so setting changes take effect immediately for new script runs; the elapsed-time comparison is promoted to `long` to avoid overflow with large values. - The Site Settings form binds the new field as a nullable `Integer`, so a POST that omits the parameter leaves the stored setting unchanged rather than silently disabling the timeout. - Added `RhinoService.TestCase.timeoutTest` (BVT) covering both a short timeout aborting a runaway script and 0 disabling the watchdog.
1 parent 39937b0 commit ec8642a

7 files changed

Lines changed: 100 additions & 3 deletions

File tree

‎api/src/org/labkey/api/settings/AppProps.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,11 @@ static WriteableAppProps getWriteableInstance()
173173
/** Timeout in seconds for read-only HTTP requests, after which resources like DB connections and spawned processes will be killed. Set to 0 to disable. */
174174
int getReadOnlyHttpRequestTimeout();
175175

176+
int DEFAULT_SCRIPT_EXECUTION_TIMEOUT = 60;
177+
178+
/** Timeout in seconds for server-side JavaScript (e.g. trigger scripts), measured in wall-clock time including database and other Java operations invoked by the script. Set to 0 to disable. */
179+
int getScriptExecutionTimeout();
180+
176181
int getMaxBLOBSize();
177182

178183
ExceptionReportingLevel getExceptionReportingLevel();

‎api/src/org/labkey/api/settings/AppPropsImpl.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,12 @@ public int getReadOnlyHttpRequestTimeout()
321321
return lookupIntValue(readOnlyHttpRequestTimeout, 0);
322322
}
323323

324+
@Override
325+
public int getScriptExecutionTimeout()
326+
{
327+
return lookupIntValue(scriptExecutionTimeout, DEFAULT_SCRIPT_EXECUTION_TIMEOUT);
328+
}
329+
324330
@Override
325331
public int getMaxBLOBSize()
326332
{

‎api/src/org/labkey/api/settings/SiteSettingsProperties.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,14 @@ public void setValue(WriteableAppProps writeable, String value)
9090
writeable.setReadOnlyHttpRequestTimeout(Integer.parseInt(value));
9191
}
9292
},
93+
scriptExecutionTimeout("Timeout in seconds for server-side JavaScript such as trigger scripts. Measured in wall-clock time, including database and other Java operations invoked by the script. Set to 0 to disable.")
94+
{
95+
@Override
96+
public void setValue(WriteableAppProps writeable, String value)
97+
{
98+
writeable.setScriptExecutionTimeout(Integer.parseInt(value));
99+
}
100+
},
93101
maxBLOBSize("Maximum file size, in bytes, to allow in database BLOBs")
94102
{
95103
@Override

‎api/src/org/labkey/api/settings/WriteableAppProps.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ public void setReadOnlyHttpRequestTimeout(int timeout)
9494
storeIntValue(readOnlyHttpRequestTimeout, timeout);
9595
}
9696

97+
public void setScriptExecutionTimeout(int timeout)
98+
{
99+
if (timeout < 0)
100+
throw new IllegalArgumentException("scriptExecutionTimeout must be >= 0");
101+
storeIntValue(scriptExecutionTimeout, timeout);
102+
}
103+
97104
public void setMaxBLOBSize(int maxSize)
98105
{
99106
if (maxSize < 0)

‎core/src/org/labkey/core/admin/AdminController.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,6 +1381,14 @@ public void validateCommand(SiteSettingsForm form, Errors errors)
13811381
{
13821382
errors.reject(ERROR_MSG, "Memory logging frequency must be non-negative");
13831383
}
1384+
if (form.getScriptExecutionTimeout() == null)
1385+
{
1386+
errors.reject(ERROR_MSG, "Script execution timeout is required; set to 0 to disable the timeout");
1387+
}
1388+
else if (form.getScriptExecutionTimeout() < 0)
1389+
{
1390+
errors.reject(ERROR_MSG, "Script execution timeout must be non-negative");
1391+
}
13841392
}
13851393

13861394
@Override
@@ -1415,6 +1423,7 @@ public boolean handlePost(SiteSettingsForm form, BindException errors) throws Ex
14151423
props.setSSLPort(form.getSslPort());
14161424
props.setMemoryUsageDumpInterval(form.getMemoryUsageDumpInterval());
14171425
props.setReadOnlyHttpRequestTimeout(form.getReadOnlyHttpRequestTimeout());
1426+
props.setScriptExecutionTimeout(form.getScriptExecutionTimeout());
14181427
props.setMaxBLOBSize(form.getMaxBLOBSize());
14191428
props.setSelfReportExceptions(form.isSelfReportExceptions());
14201429

@@ -2384,6 +2393,7 @@ public static class SiteSettingsForm
23842393
private int _sslPort;
23852394
private int _memoryUsageDumpInterval;
23862395
private int _readOnlyHttpRequestTimeout;
2396+
private Integer _scriptExecutionTimeout;
23872397
private int _maxBLOBSize;
23882398
private String _exceptionReportingLevel;
23892399
private String _usageReportingLevel;
@@ -2524,6 +2534,18 @@ public int getReadOnlyHttpRequestTimeout()
25242534
return _readOnlyHttpRequestTimeout;
25252535
}
25262536

2537+
/** Null when the request omits or blanks the parameter; rejected in validateCommand so an omitted value can never bind to 0 and silently disable the timeout. */
2538+
@Nullable
2539+
public Integer getScriptExecutionTimeout()
2540+
{
2541+
return _scriptExecutionTimeout;
2542+
}
2543+
2544+
public void setScriptExecutionTimeout(@Nullable Integer timeout)
2545+
{
2546+
_scriptExecutionTimeout = timeout;
2547+
}
2548+
25272549
public void setReadOnlyHttpRequestTimeout(int timeout)
25282550
{
25292551
_readOnlyHttpRequestTimeout = timeout;

‎core/src/org/labkey/core/admin/customizeSite.jsp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,11 @@ Click the Save button at any time to accept the current settings and continue.</
312312
"After the timeout, resources like database connections and spawned processes will be killed to abort processing the request. Set to 0 to disable the timeout.")%></label></td>
313313
<td><input type="text" name="<%=readOnlyHttpRequestTimeout%>" id="<%=readOnlyHttpRequestTimeout%>" size="4" value="<%=appProps.getReadOnlyHttpRequestTimeout()%>"></td>
314314
</tr>
315+
<tr>
316+
<td class="labkey-form-label"><label for="<%=scriptExecutionTimeout%>">Timeout for server-side scripts, in seconds<%=helpPopup("Script execution timeout",
317+
"Maximum time a server-side JavaScript invocation (such as a trigger script) may run before it is terminated. Measured in wall-clock time, including database and other Java operations invoked by the script. Set to 0 to disable the timeout.")%></label></td>
318+
<td><input type="text" name="<%=scriptExecutionTimeout%>" id="<%=scriptExecutionTimeout%>" size="4" value="<%=appProps.getScriptExecutionTimeout()%>"></td>
319+
</tr>
315320
<tr>
316321
<td class="labkey-form-label"><label for="<%=maxBLOBSize%>">Maximum file size, in bytes, to allow in database BLOBs</label></td>
317322
<td><input type="text" name="<%=maxBLOBSize%>" id="<%=maxBLOBSize%>" size="10" value="<%=appProps.getMaxBLOBSize()%>"></td>

‎core/src/org/labkey/core/script/RhinoService.java‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,15 @@
4040
import org.labkey.api.resource.Resource;
4141
import org.labkey.api.script.ScriptReference;
4242
import org.labkey.api.script.ScriptService;
43+
import org.labkey.api.security.User;
44+
import org.labkey.api.settings.AppProps;
45+
import org.labkey.api.settings.WriteableAppProps;
4346
import org.labkey.api.test.TestWhen;
4447
import org.labkey.api.util.HeartBeat;
4548
import org.labkey.api.util.JunitUtil;
4649
import org.labkey.api.util.MemTracker;
4750
import org.labkey.api.util.Path;
51+
import org.labkey.api.util.TestContext;
4852
import org.labkey.api.util.UnexpectedException;
4953
import org.labkey.api.view.HttpView;
5054
import org.mozilla.javascript.ClassShutter;
@@ -199,6 +203,44 @@ public void reportTest() throws Exception
199203
test("reportTest");
200204
}
201205

206+
@Test
207+
public void timeoutTest() throws Exception
208+
{
209+
int original = AppProps.getInstance().getScriptExecutionTimeout();
210+
User user = TestContext.get().getUser();
211+
212+
try
213+
{
214+
// Busy-loop bounded at 15 seconds of wall-clock time; the 1-second watchdog should abort it long before that. HeartBeat ticks once per second, so the abort lands after 2-3 real seconds.
215+
setScriptExecutionTimeout(1, user);
216+
try
217+
{
218+
RhinoService.RHINO_FACTORY.getScriptEngine().eval("var start = new Date().getTime(); while (new Date().getTime() - start < 15000) {}");
219+
fail("Expected script to be terminated by the execution timeout");
220+
}
221+
catch (Exception e)
222+
{
223+
assertTrue("Unexpected script error: " + e.getMessage(), e.getMessage().contains("Script execution exceeded 1 seconds"));
224+
}
225+
226+
// 0 disables the watchdog: a loop that runs well past the 1-second timeout above should complete normally
227+
setScriptExecutionTimeout(0, user);
228+
Object result = RhinoService.RHINO_FACTORY.getScriptEngine().eval("var start = new Date().getTime(); while (new Date().getTime() - start < 2500) {} 'completed';");
229+
assertEquals("completed", result);
230+
}
231+
finally
232+
{
233+
setScriptExecutionTimeout(original, user);
234+
}
235+
}
236+
237+
private void setScriptExecutionTimeout(int seconds, User user)
238+
{
239+
WriteableAppProps props = AppProps.getWriteableInstance();
240+
props.setScriptExecutionTimeout(seconds);
241+
props.save(user);
242+
}
243+
202244
private void test(String scriptName) throws ScriptException, NoSuchMethodException
203245
{
204246
Path js = Path.parse(ScriptService.SCRIPTS_DIR + "/validationTest/" + scriptName + ".js");
@@ -1006,9 +1048,8 @@ protected void observeInstructionCount(Context cx, int instructionCount)
10061048
{
10071049
SandboxContext ctx = (SandboxContext)cx;
10081050
long currentTime = HeartBeat.currentTimeMillis();
1009-
final int timeout = 60;
1010-
if (currentTime - ctx.startTime > timeout*1000)
1011-
Context.reportError("Script execution exceeded " + timeout + " seconds.");
1051+
if (ctx.timeoutSeconds > 0 && currentTime - ctx.startTime > ctx.timeoutSeconds * 1000L)
1052+
Context.reportError("Script execution exceeded " + ctx.timeoutSeconds + " seconds.");
10121053
}
10131054

10141055
@Override
@@ -1044,12 +1085,15 @@ public boolean visibleToScripts(String fullClassName)
10441085
private static class SandboxContext extends Context
10451086
{
10461087
private final long startTime;
1088+
// resolved once per context; observeInstructionCount runs far too often for a property lookup
1089+
private final int timeoutSeconds;
10471090

10481091
private SandboxContext(SandboxContextFactory factory)
10491092
{
10501093
super(factory);
10511094
setLanguageVersion(Context.VERSION_1_8);
10521095
startTime = HeartBeat.currentTimeMillis();
1096+
timeoutSeconds = AppProps.getInstance().getScriptExecutionTimeout();
10531097
}
10541098
}
10551099

0 commit comments

Comments
 (0)