msgbox in onpluginbroadcast (from gmcp handler) breaks mccp decompression

Posted by Fiendish on Sun 15 Jan 2012 02:28 AM — 4 posts, 11,952 views.

Fiendish USA Global Moderator Sun 15 Jan 2012 02:28 AM #0

Adding the following simple plugin to the Aardwolf client package causes decompression errors. Sometimes MUSHclient crashes too. (Just load the plugin, connect, and wait 20 seconds or so.)


<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE muclient>

<muclient>
<plugin
   name="Error_Test"
   author="Fiendish"
   id="aaaaaa47725d717d6292a680"
   language="Lua"
   purpose="Compression Errors"
   date_written="2013-07-05 16:46:02"
   requires="4.81"
   version="1.0"
   save_state="y"
   >
</plugin>

<script>
<![CDATA[
function OnPluginBroadcast (msg, id, name, text)
   if id == "3e7dedbe37e44942dd46d264" then -- the Aardwolf GMCP handler
      utils.msgbox("Test!")
   end -- level info
end

]]>
</script>
</muclient>

Nick Gammon Australia Forum Administrator Sun 15 Jan 2012 09:46 PM #1

Your test is setting up a race condition, with re-entrant behaviour that is not really designed to be handled.

This modified version, I had sitting there for a while (with the message box up) without a crash:


function OnPluginBroadcast (msg, id, name, text)
   if id == "3e7dedbe37e44942dd46d264" then -- the Aardwolf GMCP handler
     if not foo then
      foo = true
      utils.msgbox("Test!")
      foo = false
     end -- if
   end -- level info
end

Your test, if you leave the message box up, interrupts the processing of the incoming packet by displaying the message box. So that packet is partly processed. Then while the message box is up, a new packet arrives, and if that also causes a plugin broadcast, we have now re-entered utils.msgbox. When these eventually get dismissed (or maybe without dismissing them) as the stack unwinds, things get out of kilter.

Another possible explanation is that, while the message box is open, Windows' message queue starts filling up (eg. with incoming comms messages) and some of them get discarded, leading to decompression errors.

Fiendish USA Global Moderator Sun 15 Jan 2012 10:31 PM #2

Quote:
a new packet arrives, and if that also causes a plugin broadcast

I thought having the messagebox up halted script processing. I wouldn't have expected a new broadcast to happen.

Quote:
while the message box is open, Windows' message queue starts filling up

So utils dialogs are never safe?

Nick Gammon Australia Forum Administrator Sun 15 Jan 2012 11:27 PM #3

Fiendish said:

I thought having the messagebox up halted script processing. I wouldn't have expected a new broadcast to happen.

If you leave install my modified version (so it doesn't actually crash) you will notice that, with the message box up, messages (from the MUD) scroll underneath.

That script halted, yes. But I think the underlying design of MFC lets a "modal" dialog box actually process more messages. In fact, they implement modal boxes by simply disabling the underlying window. You notice this because things scroll by while you have the world configuration dialog box open, for example.

Effectively I think the underlying MFC design is pulling out a new incoming comms message, sending to the document class, and it then processes it in the normal way. It doesn't know the script engine is "paused" processing an earlier packet.

Probably what is crashing is the Lua script engine being re-entered.

Quote:
So utils dialogs are never safe?

Like I said, without the re-entry, I could have one dialog up for a while. But putting up dialogs in the low-level packet processing (where the GMCP packets come in) is probably asking for trouble.

My tentative suggestion would be to queue up such dialogs so they get pulled out in "main" processing, not half-way through handling a packet.

The whole thing wasn't really designed with re-entrancy in mind. And a while back a couple of things were added that tended to exacerbate this problem:

Having blocking dialogs, like utils.msgbox
Putting hooks into low-level code (like telnet subnegotiation) which allows you to call scripts, which lets you put up such dialogs.