Skip to content

Commit 5b3e40d

Browse files
committed
fix: fix setTimeout dead lock
1 parent 0e0cb70 commit 5b3e40d

File tree

4 files changed

+166
-158
lines changed

4 files changed

+166
-158
lines changed

src/legacy/api/EventAPI.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,7 @@ void EnableEventListener(int eventId) {
838838
}
839839
}
840840

841-
ll::schedule::GameTickScheduler scheduler;
841+
ll::schedule::GameTickScheduler eventScheduler;
842842

843843
void InitBasicEventListeners() {
844844
using namespace ll::event;
@@ -936,7 +936,7 @@ void InitBasicEventListeners() {
936936
// ===== onServerStarted =====
937937
bus.emplaceListener<ServerStartedEvent>([](ServerStartedEvent& ev) {
938938
using namespace ll::chrono_literals;
939-
scheduler.add<ll::schedule::DelayTask>(1_tick, [] {
939+
eventScheduler.add<ll::schedule::DelayTask>(1_tick, [] {
940940
IF_LISTENED(EVENT_TYPES::onServerStarted) { CallEventUncancelable(EVENT_TYPES::onServerStarted); }
941941
IF_LISTENED_END(EVENT_TYPES::onServerStarted);
942942
isCmdRegisterEnabled = true;
@@ -949,7 +949,7 @@ void InitBasicEventListeners() {
949949
// 植入tick
950950
using namespace ll::chrono_literals;
951951

952-
scheduler.add<ll::schedule::RepeatTask>(1_tick, [] {
952+
eventScheduler.add<ll::schedule::RepeatTask>(1_tick, [] {
953953
#ifndef LEGACY_SCRIPT_ENGINE_BACKEND_NODEJS
954954
try {
955955
std::list<ScriptEngine*> tmpList;

src/legacy/api/SystemAPI.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ Local<Value> SystemClass::cmd(const Arguments& args) {
115115
return Boolean::newBoolean(NewProcess(
116116
"cmd /c" + cmd,
117117
[callback{std::move(callbackFunc)}, engine{EngineScope::currentEngine()}](int exitCode, string output) {
118-
ll::schedule::DelayTask<ll::chrono::ServerClock>(
118+
ll::schedule::DelayTask<ll::chrono::GameTickClock>(
119119
ll::chrono::ticks(1),
120120
[engine, callback = std::move(callback), exitCode, output = std::move(output)]() {
121121
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
@@ -150,7 +150,7 @@ Local<Value> SystemClass::newProcess(const Arguments& args) {
150150
return Boolean::newBoolean(NewProcess(
151151
process,
152152
[callback{std::move(callbackFunc)}, engine{EngineScope::currentEngine()}](int exitCode, string output) {
153-
ll::schedule::DelayTask<ll::chrono::ServerClock>(
153+
ll::schedule::DelayTask<ll::chrono::GameTickClock>(
154154
ll::chrono::ticks(1),
155155
[engine, callback = std::move(callback), exitCode, output = std::move(output)]() {
156156
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;

src/legacy/engine/TimeTaskSystem.cpp

Lines changed: 141 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "engine/EngineManager.h"
55
#include "engine/EngineOwnData.h"
66
#include "engine/MessageSystem.h"
7-
#include "ll/api/schedule/Scheduler.h"
7+
#include "ll/api/schedule/scheduler.h"
88

99
#include <chrono>
1010
#include <ll/api/chrono/GameChrono.h>
@@ -15,16 +15,16 @@
1515
#include <shared_mutex>
1616
#include <vector>
1717

18-
std::atomic_uint timeTaskId = 0;
19-
std::shared_mutex locker;
20-
ll::schedule::ServerTimeScheduler scheduler;
18+
std::atomic_uint timeTaskId = 0;
19+
std::shared_mutex locker;
20+
ll::schedule::GameTickScheduler taskScheduler;
2121
struct TimeTaskData {
22-
std::shared_ptr<ll::schedule::Task<ll::chrono::ServerClock>> task;
23-
script::Global<Function> func;
24-
vector<script::Global<Value>> paras;
25-
script::Global<String> code;
26-
ScriptEngine* engine;
27-
inline void swap(TimeTaskData& rhs) {
22+
uint64 task;
23+
script::Global<Function> func;
24+
vector<script::Global<Value>> paras;
25+
script::Global<String> code;
26+
ScriptEngine* engine;
27+
inline void swap(TimeTaskData& rhs) {
2828
std::swap(rhs.task, task);
2929
std::swap(rhs.engine, engine);
3030
rhs.code.swap(code);
@@ -66,7 +66,7 @@ std::unordered_map<int, TimeTaskData> timeTaskMap;
6666
// EngineScope enter(engine);
6767
// for (auto& para : paras) tmp.emplace_back(std::move(para));
6868
// }
69-
// scheduler.add<ll::schedule::DelayTask>(
69+
// taskScheduler.add<ll::schedule::DelayTask>(
7070
// std::chrono::milliseconds(timeout),
7171
// [engine, func = std::move(func), paras = std::move(tmp)]() {
7272
// if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
@@ -92,39 +92,41 @@ int NewTimeout(Local<Function> func, vector<Local<Value>> paras, int timeout) {
9292
data.func = func;
9393
data.engine = EngineScope::currentEngine();
9494
for (auto& para : paras) data.paras.emplace_back(std::move(para));
95-
data.task = scheduler.add<ll::schedule::DelayTask>(
96-
std::chrono::milliseconds(timeout),
97-
[engine{EngineScope::currentEngine()}, id{tid}]() {
98-
try {
99-
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
100-
if (!EngineManager::isValid(engine)) return;
101-
// lock after enter EngineScope to prevent deadlock
102-
EngineScope scope(engine);
103-
TimeTaskData taskData;
104-
{
105-
std::unique_lock<std::shared_mutex> lock(locker);
106-
107-
auto t = timeTaskMap.find(id);
108-
if (t == timeTaskMap.end()) return;
109-
t->second.swap(taskData);
110-
timeTaskMap.erase(id);
111-
}
112-
113-
if (taskData.func.isEmpty()) return;
114-
auto func = taskData.func.get();
115-
if (taskData.paras.empty()) {
116-
func.call();
117-
} else {
118-
vector<Local<Value>> args;
119-
for (auto& para : taskData.paras)
120-
if (para.isEmpty()) return;
121-
else args.emplace_back(para.get());
122-
func.call({}, args);
123-
}
124-
}
125-
TIMETASK_CATCH("setTimeout-Function");
126-
}
127-
);
95+
data.task = taskScheduler
96+
.add<ll::schedule::DelayTask>(
97+
std::chrono::milliseconds(timeout),
98+
[engine{EngineScope::currentEngine()}, id{tid}]() {
99+
try {
100+
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
101+
if (!EngineManager::isValid(engine)) return;
102+
// lock after enter EngineScope to prevent deadlock
103+
EngineScope scope(engine);
104+
TimeTaskData taskData;
105+
{
106+
std::unique_lock<std::shared_mutex> lock(locker);
107+
108+
auto t = timeTaskMap.find(id);
109+
if (t == timeTaskMap.end()) return;
110+
t->second.swap(taskData);
111+
timeTaskMap.erase(id);
112+
}
113+
114+
if (taskData.func.isEmpty()) return;
115+
auto func = taskData.func.get();
116+
if (taskData.paras.empty()) {
117+
func.call();
118+
} else {
119+
std::vector<Local<Value>> args;
120+
for (auto& para : taskData.paras)
121+
if (para.isEmpty()) return;
122+
else args.emplace_back(para.get());
123+
func.call({}, args);
124+
}
125+
}
126+
TIMETASK_CATCH("setTimeout-Function");
127+
}
128+
)
129+
->getId();
128130
std::unique_lock<std::shared_mutex> lock(locker);
129131
data.swap(timeTaskMap[tid]);
130132
return tid;
@@ -137,30 +139,32 @@ int NewTimeout(Local<String> func, int timeout) {
137139
data.code = func;
138140
data.engine = EngineScope::currentEngine();
139141

140-
data.task = scheduler.add<ll::schedule::DelayTask>(
141-
std::chrono::milliseconds(timeout),
142-
[engine{EngineScope::currentEngine()}, id{tid}]() {
143-
try {
144-
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
145-
if (!EngineManager::isValid(engine)) return;
146-
EngineScope scope(engine);
147-
TimeTaskData taskData;
148-
{
149-
std::unique_lock<std::shared_mutex> lock(locker);
150-
151-
auto t = timeTaskMap.find(id);
152-
if (t == timeTaskMap.end()) return;
153-
t->second.swap(taskData);
154-
timeTaskMap.erase(id);
155-
}
156-
157-
if (taskData.code.isEmpty()) return;
158-
auto code = taskData.code.get().toString();
159-
engine->eval(code);
160-
}
161-
TIMETASK_CATCH("setTimeout-String");
162-
}
163-
);
142+
data.task = taskScheduler
143+
.add<ll::schedule::DelayTask>(
144+
std::chrono::milliseconds(timeout),
145+
[engine{EngineScope::currentEngine()}, id{tid}]() {
146+
try {
147+
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
148+
if (!EngineManager::isValid(engine)) return;
149+
EngineScope scope(engine);
150+
TimeTaskData taskData;
151+
{
152+
std::unique_lock<std::shared_mutex> lock(locker);
153+
154+
auto t = timeTaskMap.find(id);
155+
if (t == timeTaskMap.end()) return;
156+
t->second.swap(taskData);
157+
timeTaskMap.erase(id);
158+
}
159+
160+
if (taskData.code.isEmpty()) return;
161+
auto code = taskData.code.get().toString();
162+
engine->eval(code);
163+
}
164+
TIMETASK_CATCH("setTimeout-String");
165+
}
166+
)
167+
->getId();
164168

165169
std::unique_lock<std::shared_mutex> lock(locker);
166170
data.swap(timeTaskMap[tid]);
@@ -175,42 +179,44 @@ int NewInterval(Local<Function> func, vector<Local<Value>> paras, int timeout) {
175179
data.engine = EngineScope::currentEngine();
176180
for (auto& para : paras) data.paras.emplace_back(std::move(para));
177181

178-
data.task = scheduler.add<ll::schedule::RepeatTask>(
179-
std::chrono::milliseconds(timeout),
180-
[engine{EngineScope::currentEngine()}, id{tid}]() {
181-
try {
182-
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
183-
if (!EngineManager::isValid(engine)) {
184-
ClearTimeTask(id);
185-
return;
186-
}
187-
EngineScope scope(engine);
188-
Local<Value> func = Local<Value>();
189-
vector<Local<Value>> args;
190-
{
191-
std::unique_lock<std::shared_mutex> lock(locker);
192-
193-
auto t = timeTaskMap.find(id);
194-
if (t == timeTaskMap.end()) return;
195-
196-
TimeTaskData& taskData = t->second;
197-
198-
if (taskData.func.isEmpty()) return;
199-
func = taskData.func.get();
200-
if (!taskData.paras.empty()) {
201-
vector<Local<Value>> args;
202-
for (auto& para : taskData.paras)
203-
if (para.isEmpty()) return;
204-
else args.emplace_back(para.get());
205-
}
206-
}
207-
if (!func.isFunction()) return;
208-
if (args.size() > 0) func.asFunction().call({}, args);
209-
else func.asFunction().call();
210-
}
211-
TIMETASK_CATCH("setInterval-Function");
212-
}
213-
);
182+
data.task = taskScheduler
183+
.add<ll::schedule::RepeatTask>(
184+
std::chrono::milliseconds(timeout),
185+
[engine{EngineScope::currentEngine()}, id{tid}]() {
186+
try {
187+
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
188+
if (!EngineManager::isValid(engine)) {
189+
ClearTimeTask(id);
190+
return;
191+
}
192+
EngineScope scope(engine);
193+
Local<Value> func = Local<Value>();
194+
vector<Local<Value>> args;
195+
{
196+
std::unique_lock<std::shared_mutex> lock(locker);
197+
198+
auto t = timeTaskMap.find(id);
199+
if (t == timeTaskMap.end()) return;
200+
201+
TimeTaskData& taskData = t->second;
202+
203+
if (taskData.func.isEmpty()) return;
204+
func = taskData.func.get();
205+
if (!taskData.paras.empty()) {
206+
vector<Local<Value>> args;
207+
for (auto& para : taskData.paras)
208+
if (para.isEmpty()) return;
209+
else args.emplace_back(para.get());
210+
}
211+
}
212+
if (!func.isFunction()) return;
213+
if (args.size() > 0) func.asFunction().call({}, args);
214+
else func.asFunction().call();
215+
}
216+
TIMETASK_CATCH("setInterval-Function");
217+
}
218+
)
219+
->getId();
214220

215221
std::unique_lock<std::shared_mutex> lock(locker);
216222
data.swap(timeTaskMap[tid]);
@@ -224,32 +230,34 @@ int NewInterval(Local<String> func, int timeout) {
224230
data.code = func;
225231
data.engine = EngineScope::currentEngine();
226232

227-
data.task = scheduler.add<ll::schedule::RepeatTask>(
228-
std::chrono::milliseconds(timeout),
229-
[engine{EngineScope::currentEngine()}, id{tid}]() {
230-
try {
231-
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
232-
if (!EngineManager::isValid(engine)) {
233-
ClearTimeTask(id);
234-
return;
235-
}
236-
EngineScope scope(engine);
237-
std::string code;
238-
{
239-
std::unique_lock<std::shared_mutex> lock(locker);
240-
241-
auto t = timeTaskMap.find(id);
242-
if (t == timeTaskMap.end()) return;
243-
TimeTaskData& taskData = t->second;
244-
245-
if (taskData.code.isEmpty()) return;
246-
code = taskData.code.get().toString();
247-
}
248-
if (!code.empty()) engine->eval(code);
249-
}
250-
TIMETASK_CATCH("setInterval-String");
251-
}
252-
);
233+
data.task = taskScheduler
234+
.add<ll::schedule::RepeatTask>(
235+
std::chrono::milliseconds(timeout),
236+
[engine{EngineScope::currentEngine()}, id{tid}]() {
237+
try {
238+
if ((ll::getServerStatus() != ll::ServerStatus::Running)) return;
239+
if (!EngineManager::isValid(engine)) {
240+
ClearTimeTask(id);
241+
return;
242+
}
243+
EngineScope scope(engine);
244+
std::string code;
245+
{
246+
std::unique_lock<std::shared_mutex> lock(locker);
247+
248+
auto t = timeTaskMap.find(id);
249+
if (t == timeTaskMap.end()) return;
250+
TimeTaskData& taskData = t->second;
251+
252+
if (taskData.code.isEmpty()) return;
253+
code = taskData.code.get().toString();
254+
}
255+
if (!code.empty()) engine->eval(code);
256+
}
257+
TIMETASK_CATCH("setInterval-String");
258+
}
259+
)
260+
->getId();
253261

254262
std::unique_lock<std::shared_mutex> lock(locker);
255263
data.swap(timeTaskMap[tid]);
@@ -258,12 +266,11 @@ int NewInterval(Local<String> func, int timeout) {
258266

259267
bool ClearTimeTask(int id) {
260268
assert(EngineScope::currentEngine() != nullptr);
261-
TimeTaskData data;
262269
try {
263270
std::unique_lock<std::shared_mutex> lock(locker);
264271
auto it = timeTaskMap.find(id);
265272
if (it != timeTaskMap.end()) {
266-
data.swap(timeTaskMap[id]);
273+
taskScheduler.remove(timeTaskMap[id].task);
267274
timeTaskMap.erase(id);
268275
}
269276
} catch (...) {
@@ -276,18 +283,16 @@ bool ClearTimeTask(int id) {
276283

277284
void LLSERemoveTimeTaskData(ScriptEngine* engine) {
278285
// enter scope to prevent script::Global::~Global() from crashing
279-
EngineScope enter(engine);
280-
std::unordered_map<int, TimeTaskData> tmpMap;
286+
EngineScope enter(engine);
281287
try {
282288
std::unique_lock<std::shared_mutex> lock(locker);
283289
for (auto it = timeTaskMap.begin(); it != timeTaskMap.end();) {
284290
if (it->second.engine == engine) {
285-
it->second.swap(tmpMap[it->first]);
291+
taskScheduler.remove(it->second.task);
286292
it = timeTaskMap.erase(it);
287293
} else ++it;
288294
}
289295
} catch (...) {
290296
lse::getSelfPluginInstance().getLogger().info("Fail in LLSERemoveTimeTaskData");
291297
}
292-
tmpMap.clear();
293298
}

0 commit comments

Comments
 (0)