Skip to content

Commit ef92653

Browse files
committed
Fix sm exts reload crashing because it doesn't update native pointers. Automatically reloads dependent plugins.
1 parent ce8125b commit ef92653

3 files changed

Lines changed: 189 additions & 3 deletions

File tree

core/logic/ExtensionSys.cpp

Lines changed: 126 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@
3232
#include <stdlib.h>
3333

3434
#include <memory>
35+
#include <algorithm>
36+
#include <string>
37+
#include <unordered_map>
38+
#include <unordered_set>
39+
#include <vector>
3540

3641
#include "ExtensionSys.h"
3742
#include <ILibrarySys.h>
@@ -254,11 +259,129 @@ bool CLocalExtension::Reload(char *error, size_t maxlength)
254259
{
255260
if (m_pLib == NULL) // FIXME: just load it instead?
256261
return false;
257-
262+
263+
// Step 1: Build a load-order map and identify direct dependents.
264+
// We must save this before any cleanup since unloading plugins removes them from
265+
// m_Dependents via DropRefsTo.
266+
struct PluginInfo {
267+
std::string filename;
268+
PluginType type;
269+
size_t order;
270+
bool was_paused;
271+
};
272+
273+
std::unordered_map<std::string, size_t> load_order;
274+
std::unordered_set<std::string> was_running;
275+
std::vector<PluginInfo> to_reload;
276+
277+
{
278+
AutoPluginList list(scripts);
279+
for (size_t i = 0; i < list->size(); i++) {
280+
SMPlugin *plugin = list->at(i);
281+
std::string filename(plugin->GetFilename());
282+
load_order[filename] = i;
283+
284+
PluginStatus status = plugin->GetStatus();
285+
if (status == Plugin_Running || status == Plugin_Paused)
286+
was_running.insert(filename);
287+
288+
CPlugin *cp = static_cast<CPlugin *>(plugin);
289+
if (m_Dependents.find(cp) != m_Dependents.end() &&
290+
(status == Plugin_Running || status == Plugin_Paused))
291+
{
292+
to_reload.push_back({filename, plugin->GetType(), i, status == Plugin_Paused});
293+
}
294+
}
295+
}
296+
297+
// Step 2: Clear native cache entries and unbind weak refs.
298+
DropEverything();
299+
300+
// Step 3: Unload direct dependent plugins. They hold JIT-baked stale function
301+
// pointers that will crash if called after dlclose.
302+
// Copy and clear m_Dependents first — UnloadPlugin triggers OnPluginDestroyed
303+
// which calls DropRefsTo, removing entries from m_Dependents during iteration.
304+
// (UnloadExtension avoids this by removing itself from m_Libs first, but we
305+
// can't do that since we need to stay in m_Libs for reload.)
306+
List<CPlugin *> dependents_copy = m_Dependents;
307+
m_Dependents.clear();
308+
for (List<CPlugin *>::iterator p_iter = dependents_copy.begin();
309+
p_iter != dependents_copy.end();
310+
p_iter++)
311+
{
312+
scripts->UnloadPlugin((*p_iter));
313+
}
314+
315+
// Step 3b: Collect and unload cascaded victims — plugins that entered Plugin_Error
316+
// because they depended on a plugin we just unloaded (not on this extension directly).
317+
bool found;
318+
do {
319+
found = false;
320+
AutoPluginList list(scripts);
321+
for (size_t i = 0; i < list->size(); i++) {
322+
SMPlugin *plugin = list->at(i);
323+
std::string filename(plugin->GetFilename());
324+
if (plugin->GetStatus() == Plugin_Error && was_running.count(filename)) {
325+
auto it = load_order.find(filename);
326+
size_t order = (it != load_order.end()) ? it->second : SIZE_MAX;
327+
to_reload.push_back({filename, plugin->GetType(), order, false});
328+
was_running.erase(filename);
329+
scripts->UnloadPlugin(plugin);
330+
found = true;
331+
break; // List was modified, restart scan.
332+
}
333+
}
334+
} while (found);
335+
336+
// Sort by original load order so inter-plugin dependencies resolve correctly.
337+
std::sort(to_reload.begin(), to_reload.end(),
338+
[](const PluginInfo &a, const PluginInfo &b) {
339+
return a.order < b.order;
340+
});
341+
342+
// Step 4: Clean up extension state to prevent duplicates on reload.
343+
g_ShareSys.RemoveInterfaces(this);
344+
for (List<String>::iterator s_iter = m_Libraries.begin();
345+
s_iter != m_Libraries.end();
346+
s_iter++)
347+
{
348+
scripts->OnLibraryAction((*s_iter).c_str(), LibraryAction_Removed);
349+
}
350+
m_Libraries.clear();
351+
m_Interfaces.clear();
352+
353+
// Step 5: Unload the extension (dlclose).
258354
m_pAPI->OnExtensionUnload();
259355
Unload();
260-
261-
return Load(error, maxlength);
356+
357+
// Step 6: Reload the extension (dlopen). This calls OnExtensionLoad which
358+
// re-registers natives, interfaces, and libraries.
359+
if (!Load(error, maxlength))
360+
return false;
361+
362+
// Step 7: Batch reload dependent plugins in original m_plugins order.
363+
// Uses two-pass loading (compile all, then resolve dependencies) so that
364+
// inter-plugin dependencies — including circular ones — resolve the same
365+
// way they do during initial load.
366+
std::vector<std::pair<std::string, PluginType>> batch;
367+
for (auto &info : to_reload)
368+
batch.push_back({info.filename, info.type});
369+
370+
std::vector<CPlugin *> results = g_PluginSys.LoadPluginBatch(batch);
371+
372+
for (size_t i = 0; i < to_reload.size(); i++) {
373+
if (!results[i]) {
374+
rootmenu->ConsolePrint("[SM] Failed to reload plugin \"%s\"",
375+
to_reload[i].filename.c_str());
376+
} else {
377+
rootmenu->ConsolePrint("[SM] Reloaded plugin \"%s\"",
378+
to_reload[i].filename.c_str());
379+
if (to_reload[i].was_paused)
380+
results[i]->SetPauseState(true);
381+
}
382+
}
383+
384+
return true;
262385
}
263386

264387
bool CRemoteExtension::IsExternal()

core/logic/PluginSys.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,6 +1084,58 @@ void CPluginManager::LoadAll_SecondPass()
10841084
m_AllPluginsLoaded = true;
10851085
}
10861086

1087+
std::vector<CPlugin *> CPluginManager::LoadPluginBatch(
1088+
const std::vector<std::pair<std::string, PluginType>> &plugins)
1089+
{
1090+
std::vector<CPlugin *> results(plugins.size(), nullptr);
1091+
1092+
// First pass: compile and prepare all plugins.
1093+
for (size_t i = 0; i < plugins.size(); i++) {
1094+
auto &[filename, type] = plugins[i];
1095+
CPlugin *pl;
1096+
LoadRes res = LoadPlugin(&pl, filename.c_str(), true, type);
1097+
if (res == LoadRes_Failure) {
1098+
g_Logger.LogError("[SM] Failed to load plugin \"%s\": %s",
1099+
filename.c_str(), pl->GetErrorMsg());
1100+
delete pl;
1101+
continue;
1102+
}
1103+
if (res == LoadRes_AlreadyLoaded) {
1104+
results[i] = pl;
1105+
continue;
1106+
}
1107+
if (res == LoadRes_NeverLoad) {
1108+
continue;
1109+
}
1110+
AddPlugin(pl);
1111+
results[i] = pl;
1112+
}
1113+
1114+
// Second pass: resolve dependencies and call OnPluginStart for all
1115+
// newly loaded plugins. This matches the LoadAll_SecondPass pattern
1116+
// where all plugins are present in m_plugins before any RunSecondPass.
1117+
for (size_t i = 0; i < results.size(); i++) {
1118+
CPlugin *pl = results[i];
1119+
if (!pl || pl->GetStatus() != Plugin_Loaded)
1120+
continue;
1121+
if (!RunSecondPass(pl)) {
1122+
g_Logger.LogError("[SM] Unable to load plugin \"%s\": %s",
1123+
pl->GetFilename(), pl->GetErrorMsg());
1124+
Purge(pl);
1125+
pl->FinishEviction();
1126+
results[i] = nullptr;
1127+
}
1128+
}
1129+
1130+
// Final pass: OnAllPluginsLoaded.
1131+
for (auto *pl : results) {
1132+
if (pl && pl->GetStatus() <= Plugin_Paused)
1133+
pl->Call_OnAllPluginsLoaded();
1134+
}
1135+
1136+
return results;
1137+
}
1138+
10871139
bool CPluginManager::FindOrRequirePluginDeps(CPlugin *pPlugin)
10881140
{
10891141
struct _pl

core/logic/PluginSys.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,17 @@ class CPluginManager :
436436
void _SetPauseState(CPlugin *pPlugin, bool pause);
437437

438438
void ForEachPlugin(ke::Function<void(CPlugin *)> callback);
439+
440+
/**
441+
* Batch-loads plugins using the two-pass approach (compile all, then
442+
* run second pass for all) so that inter-plugin dependencies — including
443+
* circular ones — resolve the same way they do during initial load.
444+
*
445+
* Returns a vector of CPlugin pointers in the same order as the input.
446+
* Entries are nullptr for plugins that failed to load.
447+
*/
448+
std::vector<CPlugin *> LoadPluginBatch(
449+
const std::vector<std::pair<std::string, PluginType>> &plugins);
439450
private:
440451
LoadRes LoadPlugin(CPlugin **pPlugin, const char *path, bool debug, PluginType type);
441452

0 commit comments

Comments
 (0)