If a plugin script section calls LoadPlugin before the world file is finished loading, it leads to an access violation reading the world file during startup. Would a possible solution be reading the plugin list fully and closing the world file before loading the plugins?
world file access violation if LoadPlugin called during client startup
Posted by Fiendish on Thu 07 Dec 2017 08:30 PM — 16 posts, 61,983 views.
Follow-on bug.
Trying to work around the above, calling DoAfterSpecial(n, 'LoadPlugin(GetInfo(60).."myplugin.xml"', 12) instead of just LoadPlugin before the world is done loading causes a crash after n seconds.
Trying to work around the above, calling DoAfterSpecial(n, 'LoadPlugin(GetInfo(60).."myplugin.xml"', 12) instead of just LoadPlugin before the world is done loading causes a crash after n seconds.
You might wonder why LoadPlugin is being called so early. It's because I have it called from a Lua module as a way of adding a global alias and global OnPluginListChanged receiver (by way of plugin) if any plugin loads that module (because any OnPluginListChanged added inside the module might just get replaced by any defined in the plugin). It works fine while MUSHclient is up and running, but then throws the access violation errors during world startup when any requires are done at the top of a plugin script section as is fairly customary.
Is this function call being done in the OnPluginInstall function, or outside any function (in global space)? That would change the execution order slightly.
Most people put requires at or near the beginning of the script section and don't fully understand the load and callback sequence, so I'm trying to make something that doesn't require redoing any logic in an existing plugin. So it could be called in OnPluginInstall, but it could also be called in the global space. I want to be able to say to someone "just put 'require foo' at the very beginning of your plugin's script section" so that they can use the contained methods and variables immediately.
I think I've managed to get around the errors by mimicing the reload plugin from inside itself thread (but wow is it convoluted):
I think I've managed to get around the errors by mimicing the reload plugin from inside itself thread (but wow is it convoluted):
local action = [[DoAfterSpecial(0.1, 'require \'checkplugin\';do_plugin_check_now(\']]..theme_controller..[[\', \'aard_Theme_Controller\')', sendto.script)]]
local prefix = GetAlphaOption("script_prefix")
action = [[
SetAlphaOption("script_prefix", "/")
Execute("/]]..action:gsub("\\", "\\\\")..[[")
SetAlphaOption("script_prefix", "]]..prefix:gsub("\\", "\\\\")..[[")
]]
DoAfterSpecial(0.1, action, sendto.script)
Can I clarify your workflow here?
You mention "require" so I presume you must have something like this:
Plugin_A (amongst others):
something.lua:
myplugin.xml:
You mention "require" so I presume you must have something like this:
Plugin_A (amongst others):
require "something.lua"
something.lua:
LoadPlugin(GetInfo(60).."myplugin.xml")
-- add a global alias to do something (what?)
myplugin.xml:
function OnPluginListChanged ()
-- handle plugin list changing
end -- OnPluginListChanged
I can't reproduce the crash by doing this:
This plugin is loaded already:
This one is not loaded (the above plugin loads it):
This plugin is loaded already:
<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE muclient>
<muclient>
<plugin
name="load_plugin_test"
author="Nick Gammon"
id="d3ecfab22afdeedb06b872f1"
language="Lua"
purpose="Tests a plugin loading another one"
date_written="2017-12-08"
requires="5.05"
version="1.0"
>
</plugin>
<script>
<![CDATA[
function OnPluginInstall ()
ColourNote ("pink", "", "In OnPluginInstall inside plugin load_plugin_test")
local result = LoadPlugin(GetInfo(60).."myplugin.xml")
ColourNote ("pink", "", "Done load of myplugin.xml, got result " ..
tostring (result) ..
" (" .. error_desc [result] .. ")")
end -- OnPluginInstall
]]>
</script>
</muclient>
This one is not loaded (the above plugin loads it):
<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE muclient>
<muclient>
<plugin
name="myplugin"
author="Nick Gammon"
id="1003ceffa695a1b0d11505c9"
language="Lua"
purpose="Tests a plugin being loaded by another one"
date_written="2017-12-08"
requires="5.05"
version="1.0"
>
</plugin>
<script>
<![CDATA[
function OnPluginInstall ()
ColourNote ("pink", "", "In OnPluginInstall inside plugin myplugin.xml")
end -- OnPluginInstall
]]>
</script>
</muclient>
I propose that trying to load a plugin (see first post) that is already loaded shouldn't raise an error. And maybe that the xml file should be read and closed before plugins are begun loading.
See: https://imgur.com/a/b4TnH
See: https://imgur.com/a/b4TnH
I don't quite see why you are getting a sharing violation on the main world file. When plugins are loaded they load with readOnly and shareDenyWrite which shouldn't cause an issue.
https://github.com/nickgammon/mushclient/blob/master/xml/xml_load_world.cpp#L875
Why would you get a sharing violation on opening the world file?
As far as I can see from MFC the main world document is also opened in the same way:
To be honest, the loading code is quite complex, and indeed the main world file is opened in MFC (as above) so I don't really have the option of closing it in advance of loading plugins.
Are you suggesting that it should silently ignore attempts to reload the same plugin?
https://github.com/nickgammon/mushclient/blob/master/xml/xml_load_world.cpp#L875
Why would you get a sharing violation on opening the world file?
As far as I can see from MFC the main world document is also opened in the same way:
BOOL CDocument::OnOpenDocument(LPCTSTR lpszPathName)
{
if (IsModified())
TRACE0("Warning: OnOpenDocument replaces an unsaved document.\n");
CFileException fe;
CFile* pFile = GetFile(lpszPathName,
CFile::modeRead|CFile::shareDenyWrite, &fe);
To be honest, the loading code is quite complex, and indeed the main world file is opened in MFC (as above) so I don't really have the option of closing it in advance of loading plugins.
Quote:
I propose that trying to load a plugin (see first post) that is already loaded shouldn't raise an error.
I propose that trying to load a plugin (see first post) that is already loaded shouldn't raise an error.
Are you suggesting that it should silently ignore attempts to reload the same plugin?
Quote:
Are you suggesting that it should silently ignore attempts to reload the same plugin?
Are you suggesting that it should silently ignore attempts to reload the same plugin?
What about something like...
- if (m_CurrentPlugin->m_strID == p->m_strID)
- ThrowErrorException ("The plugin '%s' is already loaded.", p->m_strName);
+ if ((m_CurrentPlugin->m_strID == p->m_strID) && (m_CurrentPlugin->m_strName != p->m_strName))
+ ThrowErrorException ("The UID of plugin '%s' conflicts with plugin '%s'.", m_CurrentPlugin->m_strName, p->m_strName);
or
- if (m_CurrentPlugin->m_strID == p->m_strID)
- ThrowErrorException ("The plugin '%s' is already loaded.", p->m_strName);
+ if (m_CurrentPlugin->m_strID == p->m_strID) {
+ if (m_CurrentPlugin->m_strName != p->m_strName)
+ ThrowErrorException ("The UID of plugin '%s' conflicts with plugin '%s'.", m_CurrentPlugin->m_strName, p->m_strName);
+ return;
+ }
There's quite a complex call chain leading to plugins being loaded. We would have to back out of that for the duplicate plugin to some appropriate point.
For example, I think your change would fail on these lines:
For example, I think your change would fail on these lines:
// if they really wanted a plugin, warn them if none found
if ((iMask & XML_PLUGINS) && !m_CurrentPlugin)
ThrowErrorException ("No plugin found");
Quote:
Why would you get a sharing violation on opening the world file?
Why would you get a sharing violation on opening the world file?
Probably because loading a new plugin tries to write to the file.
Why would a plugin write to the world file? Is it trying to do a world save during initialization?
Does LoadPLugin not add the plugin entry to the bottom of the world file?
No, not directly. That would be part of the world saving process.
I did an empirical test. Calling LoadPlugin does not change the date/time of the underlying world file. In other words, it does not cause the file to be written.