Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double plugin unload crash on meta retry #204

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

GAMMACASE
Copy link
Member

Prevent double plugin unload on meta retry if the plugin has failed to load on that retry attempt.

Current retry logic checks for plugin load state after a call to _Load, and unloads the plugin if it failed to load

CPlugin *pl = _Load((*i)->m_File.c_str(), Pl_Console, error, len);
if (!pl)
{
return false;
}
if (pl->m_Status >= Pl_Paused)
{
//Now it gets crazy... unload the original copy.
_Unload( (*i), true, buffer, sizeof(buffer)-1 );
//Set the new copy's id
pl->m_Id = id;
//We just wasted an id... reclaim it
m_LastId--;
return true;
}
else
{
//don't really care about the buffer here
_Unload(pl, true, buffer, sizeof(buffer)-1);
//We just wasted an id... reclaim it
m_LastId--;
return false;
}

But _Load method by itself checks for plugin load state and performs unload if it was unsuccessful
if (pl->m_Lib && (pl->m_Status < Pl_Paused))
{
pl->m_Events.clear();
UnregAllConCmds(pl);
g_SourceHook.UnloadPlugin(pl->m_Id, new Unloader(pl, false));
}

This causes a double unload if a plugin has failed to load on that retry attempt, thus causing ReadyToUnload to be called twice, where on a second go, plugin_->m_Lib would be nullptr and results in a crash on linux
void ReadyToUnload(SourceHook::Plugin plug)
{
if (plugin_->m_UnloadFn != NULL)
plugin_->m_UnloadFn();
dlclose(plugin_->m_Lib);
if (destroy_)
{
delete plugin_;
}
else
{
plugin_->m_Lib = NULL;
plugin_->m_API = NULL;
}
delete this;
}

This pr both, removes the double unload on retry and adds a safeguard on dlclose in ReadyToUnload method to check for potential nullptr.

@GAMMACASE GAMMACASE merged commit a4f4f0e into alliedmodders:master Feb 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants