Skip to content

Commit c996acd

Browse files
committed
Fixed a bug where occasionally evaluate() would last the full timeout even though execution already finished.
1 parent a52a08b commit c996acd

File tree

1 file changed

+18
-12
lines changed

1 file changed

+18
-12
lines changed

context.cpp

+18-12
Original file line numberDiff line numberDiff line change
@@ -161,39 +161,39 @@ Php::Value Context::evaluate(Php::Parameters &params)
161161
v8::Local<v8::String> source(v8::String::NewFromUtf8(Isolate::get(), params[0]));
162162
v8::Local<v8::Script> script(v8::Script::Compile(source));
163163

164-
// we create an atomic_flag and condition_variable so we can use wait_until on
164+
// we create a mutex and a condition_variable so we can use wait_until on
165165
// another thread which we can stop from our main thread. We use this to maybe abort
166166
// execution of javascript after a certain amount of time.
167-
std::atomic_flag busy = ATOMIC_FLAG_INIT;
167+
bool busy = true;
168+
std::mutex mutex;
168169
std::condition_variable condition;
169170

170-
// we are not yet finished
171-
busy.test_and_set();
172-
173171
// create a temporary thread which will mostly just sleep, but kill the script after a certain time period
174172
std::thread aborter;
175173

176174
// only create this thread if our timeout is higher than 0
177-
if (timeout > 0) aborter = std::move(std::thread([this, &busy, &condition, timeout]() {
178-
179-
// create a lock
180-
std::mutex mutex;
181-
std::unique_lock<std::mutex> lock(mutex);
175+
if (timeout > 0) aborter = std::move(std::thread([this, &busy, &mutex, &condition, timeout]() {
182176

183177
// time we want to terminate execution
184178
auto end = std::chrono::system_clock::now() + std::chrono::seconds(timeout);
185179

186180
// has the execution timed out?
187181
std::cv_status status = std::cv_status::no_timeout;
188182

183+
// obtain a lock around busy
184+
std::unique_lock<std::mutex> lock(mutex);
185+
189186
// check to prevent a spurious wakeup from removing the timeout
190187
// additionally, the main thread might have finished before we even start
191-
while (busy.test_and_set() && status != std::cv_status::timeout)
188+
while (busy && status != std::cv_status::timeout)
192189
{
193190
// we wait until some point in the future (this unlocks the lock until it returns)
194191
status = condition.wait_until(lock, end);
195192
}
196193

194+
// unlock the lock
195+
lock.unlock();
196+
197197
// in case we timeout we must terminate execution
198198
if (status != std::cv_status::timeout) return;
199199

@@ -207,8 +207,14 @@ Php::Value Context::evaluate(Php::Parameters &params)
207207
// execute the script
208208
v8::Local<v8::Value> result(script->Run());
209209

210+
// obtain a lock around busy
211+
std::unique_lock<std::mutex> lock(mutex);
212+
210213
// we are no longer busy
211-
busy.clear();
214+
busy = false;
215+
216+
// unlock the lock
217+
lock.unlock();
212218

213219
// notify the aborter thread
214220
condition.notify_one();

0 commit comments

Comments
 (0)