Fix crash when event queue is full (#521)

* each SpeechEvent needs to contain a copy of the speechtokens

* added core changes

* prevent memoryleak

* add basic pol test

* Remove extraneous prints in test

* Address review comments
- Refactor token encoding into a method

---------

Co-authored-by: Kevin Eady <8634912+KevinEady@users.noreply.github.com>

docs/docs.polserver.com/pol100/corechanges.xml

@@ -2,9 +2,14 @@
 <ESCRIPT>
 	<header>
 		<topic>Latest Core Changes</topic>
-		<datemodified>04-23-2023</datemodified>
+		<datemodified>05-04-2023</datemodified>
 	</header>
 	<version name="POL100.1.0">
+		<entry>
+			<date>04-04-2023</date>
+			<author>Turley:</author>
+			<change type="Fixed">Crash when EventQueue is full due to SpeechEvents</change>
+		</entry>
 		<entry>
 			<date>04-23-2023</date>
 			<author>Kevin:</author>

pol-core/doc/core-changes.txt

@@ -1,4 +1,6 @@
 -- POL100.1.0 --
+04-04-2023 Turley:
+    Fixed: Crash when EventQueue is full due to SpeechEvents
 04-23-2023 Kevin:
     Added: PackJSON() now accepts an option for `prettify`. If true, the output string will be indented with new lines.
 03-19-2023 Kevin:

pol-core/pol/speech.cpp

@@ -86,7 +86,7 @@ void handle_processed_speech( Network::Client* client, const std::string& text,
   {
     INFO_PRINT << chr->name() << " speaking w/ color 0x" << fmt::hexu( cfBEu16( color ) ) << "\n";
   }
-  std::string convertedText = Clib::strUtf8ToCp1252(text);
+  std::string convertedText = Clib::strUtf8ToCp1252( text );
   u16 textlen = static_cast<u16>( convertedText.size() + 1 );
   if ( textlen > SPEECH_MAX_LEN + 1 )
     textlen = SPEECH_MAX_LEN + 1;
@@ -98,7 +98,7 @@ void handle_processed_speech( Network::Client* client, const std::string& text,
   talkmsg->Write<u8>( type );  // FIXME authorize
   talkmsg->WriteFlipped<u16>( textcol );
   talkmsg->WriteFlipped<u16>( font );
-  talkmsg->Write( Clib::strUtf8ToCp1252(chr->name()).c_str(), 30 );
+  talkmsg->Write( Clib::strUtf8ToCp1252( chr->name() ).c_str(), 30 );
   talkmsg->Write( convertedText.c_str(), textlen );
   u16 len = talkmsg->offset;
   talkmsg->offset = 1;
@@ -206,7 +206,7 @@ void SpeechHandler( Network::Client* client, PKTIN_03* mymsg )
 }
 
 void SendUnicodeSpeech( Network::Client* client, PKTIN_AD* msgin, const std::string& text,
-                        Bscript::ObjArray* speechtokens )
+                        std::unique_ptr<Bscript::ObjArray> speechtokens )
 {
   if ( text.empty() )
     return;
@@ -260,7 +260,7 @@ void SendUnicodeSpeech( Network::Client* client, PKTIN_AD* msgin, const std::str
   talkmsg->WriteFlipped<u16>( textcol );
   talkmsg->WriteFlipped<u16>( msgin->font );
   talkmsg->Write( msgin->lang, 4 );
-  talkmsg->Write( Clib::strUtf8ToCp1252(chr->name()).c_str(), 30 );
+  talkmsg->Write( Clib::strUtf8ToCp1252( chr->name() ).c_str(), 30 );
 
   std::vector<u16> utf16 = Bscript::String::toUTF16( text );
   if ( utf16.size() > SPEECH_MAX_LEN )
@@ -367,7 +367,7 @@ void SendUnicodeSpeech( Network::Client* client, PKTIN_AD* msgin, const std::str
           [&]( Mobile::Character* otherchr )
           {
             Mobile::NPC* npc = static_cast<Mobile::NPC*>( otherchr );
-            npc->on_pc_spoke( chr, text, msgin->type, msgin->lang, speechtokens );
+            npc->on_pc_spoke( chr, text, msgin->type, msgin->lang, speechtokens.get() );
           } );
     }
     else
@@ -377,11 +377,11 @@ void SendUnicodeSpeech( Network::Client* client, PKTIN_AD* msgin, const std::str
           [&]( Mobile::Character* otherchr )
           {
             Mobile::NPC* npc = static_cast<Mobile::NPC*>( otherchr );
-            npc->on_ghost_pc_spoke( chr, text, msgin->type, msgin->lang, speechtokens );
+            npc->on_ghost_pc_spoke( chr, text, msgin->type, msgin->lang, speechtokens.get() );
           } );
     }
     ListenPoint::sayto_listening_points( client->chr, text, msgin->type, msgin->lang,
-                                         speechtokens );
+                                         speechtokens.get() );
   }
 }
 u16 Get12BitNumber( u8* thearray, u16 theindex )
@@ -439,7 +439,7 @@ void UnicodeSpeechHandler( Network::Client* client, PKTIN_AD* msgin )
     msgin->type &= ( ~0xC0 );  // Client won't accept C0 text type messages, so must set to 0
   }
 
-  SendUnicodeSpeech( client, msgin, text, speechtokens.release() );
+  SendUnicodeSpeech( client, msgin, text, std::move( speechtokens ) );
 }
 }  // namespace Core
 }  // namespace Pol

pol-core/pol/uoscrobj.cpp

@@ -4743,7 +4743,7 @@ SpeechEvent::SpeechEvent( Mobile::Character* speaker, const std::string& speech,
   if ( !lang.empty() )
     addMember( "langcode", new String( lang ) );
   if ( speechtokens != nullptr )
-    addMember( "tokens", speechtokens );
+    addMember( "tokens", new Bscript::ObjArray( *speechtokens ) );
 }
 
 DamageEvent::DamageEvent( Mobile::Character* source, unsigned short damage )

testsuite/pol/testpkgs/client/test_client.src

@@ -54,6 +54,94 @@ exported function say_something()
   return 1;
 endfunction
 
+exported function say_something_with_tokens()
+  var arg := struct{
+    text := "Hello Test",
+    tokens:= {60, 255}
+  };
+
+  // This needs to match what gets echo'ed from test_speech's NPC script ai_speech.src
+  var expected_message := $"source=[npc] text=[{arg.text}] tokens=[{arg.tokens}]";
+
+  var npc :=  CreateNpcFromTemplate(":testnpc:test_speech", char.x + 1, char.y + 1, char.z, realm := char.realm, forcelocation := 1);
+  if (!npc)
+    return ret_error($"Could not create npc: {npc.errortext}");
+  endif
+
+  Clear_Event_Queue();
+  clientcon.sendevent(struct{
+    todo := "speech",
+    arg := arg,
+    id := 0
+  });
+
+  while (1)
+    var ev:=waitForClient(0, {EVT_SPEECH});
+    if (!ev)
+      MoveObjectToLocation(npc, 80, 80, 0, flags := MOVEOBJECT_FORCELOCATION);
+      npc.kill();
+      return ev;
+    endif
+
+    if (ev.msg == expected_message)
+      break;
+    endif
+  endwhile
+
+  MoveObjectToLocation(npc, 80, 80, 0, flags := MOVEOBJECT_FORCELOCATION);
+  npc.kill();
+  return 1;
+endfunction
+
+exported function test_speechtoken_crash()
+  var arg := struct{
+    text := "Hello Test",
+    tokens:= {60, 255}
+  };
+
+  // This needs to match what gets echo'ed from EchoItem's control script control_echo.src
+  var expected_message := $"source=[item] text=[{arg.text}] tokens=[{arg.tokens}]";
+
+  var npc := CreateNpcFromTemplate(":testnpc:test_speech", char.x + 1, char.y + 1, char.z, override_properties := struct{ CProps := dictionary{ "EventSize" -> 0 } }, realm := char.realm, forcelocation := 1);
+  if (!npc)
+    return ret_error($"Could not create npc: {npc.errortext}");
+  endif
+
+  var item :=  CreateItemAtLocation( char.x+2, char.y+2, char.z, "EchoItem", realm := char.realm );
+  if (!item)
+    MoveObjectToLocation(npc, 80, 80, 0, flags := MOVEOBJECT_FORCELOCATION);
+    npc.kill();
+    return ret_error($"Could not create item: {item.errortext}");
+  endif
+
+  Clear_Event_Queue();
+  clientcon.sendevent(struct{
+    todo := "speech",
+    arg := arg,
+    id := 0
+  });
+
+  while (1)
+    var ev:=waitForClient(0, {EVT_SPEECH});
+    if (!ev)
+      MoveObjectToLocation(npc, 80, 80, 0, flags := MOVEOBJECT_FORCELOCATION);
+      npc.kill();
+      DestroyItem(item);
+      return ev;
+    endif
+
+    // NPC handles events first, then items.
+    if (ev.msg == expected_message)
+      break;
+    endif
+  endwhile
+
+  MoveObjectToLocation(npc, 80, 80, 0, flags := MOVEOBJECT_FORCELOCATION);
+  npc.kill();
+  DestroyItem(item);
+  return 1;
+endfunction
+
 exported function move_turn_water()
   MoveObjectToLocation(char, 1,1,1,flags:=MOVEOBJECT_FORCELOCATION);
   char.facing:=1;

testsuite/pol/testpkgs/item/control_echo.src

@@ -0,0 +1,18 @@
+use os;
+use uo;
+
+const SYSEVENT_SPEECH := 0x00000001;
+
+program main(item)
+  RegisterForSpeechEvents( item, 12 );
+  while (1)
+    var ev := os::wait_for_event(30);
+    case(ev.type)
+      SYSEVENT_SPEECH:
+        var tokens := ev.tokens;
+        tokens.sort();
+        // This needs to match what's tested in test_client's say_something_with_tokens
+        PrintTextAbove(item, $"source=[item] text=[{ev.text}] tokens=[{tokens}]");
+    endcase
+  endwhile
+endprogram

testsuite/pol/testpkgs/item/itemdesc.cfg

@@ -35,3 +35,10 @@ Item 0x300001
   Graphic 0x1
   MethodScript :TestItems:method
 }
+
+Item 0x400001
+{
+  Name EchoItem
+  Graphic 0x1
+  ControlScript control_echo
+}

testsuite/pol/testpkgs/npc/ai_speech.src

@@ -0,0 +1,30 @@
+use npc;
+use os;
+use uo;
+
+const SYSEVENT_SPEECH := 0x00000001;
+
+program ai()
+  EnableEvents(SYSEVENT_SPEECH, 12);
+
+  var queue_size := GetObjProperty(Self(), "EventSize");
+  if (queue_size != error)
+    Set_Event_Queue_Size(queue_size);
+  endif
+
+  while (1)
+    // We use wait_for_event(0) with Sleep() because events are not queued if
+    // the script is _actively_ waiting for events. This forces queuing in order to
+    // test issues with queued events.
+    var ev := os::wait_for_event(0);
+
+    case(ev.type)
+      SYSEVENT_SPEECH:
+        var tokens := ev.tokens;
+        tokens.sort();
+        // This needs to match what's tested in test_client's say_something_with_tokens
+        Say($"source=[npc] text=[{ev.text}] tokens=[{tokens}]");
+    endcase
+    Sleep(1);
+  endwhile
+endprogram

testsuite/pol/testpkgs/npc/npcdesc.cfg

@@ -24,6 +24,31 @@ NPCTemplate test_movement
     AttackDamage        101d5
 }
 
+NPCTemplate test_speech
+{
+    Name                testNPC
+    Script              ai_speech
+    MoveMode            L
+
+    ObjType             0x190
+    Color               1002
+    TrueColor           1002
+    Gender              0
+    STR                 200
+    INT                 200
+    DEX                 200
+    HITS                200
+    MANA                200
+    STAM                200
+
+    Archery             110
+    Privs               invul
+    Settings            invul
+    AttackAttribute     Wrestling
+    AttackSpeed         80
+    AttackDamage        101d5
+}
+
 
 NPCTemplate test_enteredarea
 {

testsuite/testclient/pyuo/pyuo/client.py

@@ -1124,14 +1124,15 @@ def doubleClick(self, obj):
     self.queue(po)
 
   @logincomplete
-  def say(self, text, font=3, color=0):
+  def say(self, text, font=3, color=0, tokens=None):
     ''' Say something, in unicode
     @param text string: Any unicode string
     @param font int: Font code, usually 3
     @param colot int: Font color, usually 0
+    @param tokens list of ints
     '''
     po = packets.UnicodeSpeechRequestPacket()
-    po.fill(po.TYP_NORMAL, self.LANG, text, color, font)
+    po.fill(po.TYP_NORMAL, self.LANG, text, color, font, tokens)
     self.queue(po)
 
   @logincomplete

testsuite/testclient/pyuo/pyuo/packets.py

@@ -1136,33 +1136,72 @@ class UnicodeSpeechRequestPacket(Packet):
   TYP_ALLIANCE = 0x0e
   ## Command Prompts
   TYP_COMMAND = 0x0f
+  ## Encoded with speech tokens
+  TYP_ENCODED = 0xc0
 
   cmd = 0xad
 
-  def fill(self, type, lang, text, color, font):
+  def fill(self, type, lang, text, color, font, tokens=None):
     '''!
     @param type int: Speech type, see TYP_ constants
     @param lang string: Three letter language code
     @param text string: What to say
     @param color int: Color code
     @param font int: Font code
+    @param tokens list of ints: speech.mul ids
     '''
     self.type = type
     self.lang = lang
     self.text = text
     self.color = color
     self.font = font
-    self.length = 1 + 2 + 1 + 2 + 2 + 4 + len(self.text)*2+2
+    self.tokens = tokens
+
+    self.length = 1 + 2 + 1 + 2 + 2 + 4
+    if tokens:
+      token_byte_length = ((((1 + len(tokens)) * 12) + 7) & (-8)) / 8
+      self.length = self.length + token_byte_length + len(self.text)+1
+    else:
+      self.length = self.length + len(self.text)*2+2
 
   def encodeChild(self):
     self.eulen()
-    self.euchar(self.type)
+    if self.tokens:
+      self.euchar(self.type | UnicodeSpeechRequestPacket.TYP_ENCODED)
+    else:
+      self.euchar(self.type)
     self.eushort(self.color)
     self.eushort(self.font)
     assert len(self.lang) == 3
     self.estring(self.lang, 4)
-    self.estring(self.text, len(self.text) + 1, True)
-
+    if self.tokens:
+      self.encodeTokens()
+    else:
+      self.estring(self.text, len(self.text) + 1, True)
+
+  def encodeTokens(self):
+    code_bytes = []
+    length = len(self.tokens)
+    code_bytes.append(length >> 4)
+    num3 = length & 15
+    flag = False
+    index = 0
+    while index < length:
+      keyword_id = self.tokens[index]
+      if flag:
+        code_bytes.append(keyword_id >> 4)
+        num3 = keyword_id & 15
+      else:
+        code_bytes.append(((num3 << 4) | ((keyword_id >> 8) & 15)))
+        code_bytes.append(keyword_id)
+      index = index + 1
+      flag = not flag
+    if not flag:
+      code_bytes.append(num3 << 4)
+
+    for code_byte in code_bytes:
+      self.euchar(code_byte)
+    self.estring(self.text, len(self.text) + 1)
 
 class UnicodeSpeechPacket(Packet):
   ''' Receive an unicode speech '''

testsuite/testclient/pyuo/testclient.py

@@ -62,7 +62,10 @@ def processTodos(self):
         self.client.addTodo(brain.Event(brain.Event.EVT_EXIT))
         return False
       elif todo=="speech":
-        self.client.say(arg)
+        if isinstance(arg, str):
+          self.client.say(arg)
+        else:
+          self.client.say(arg['text'], tokens = arg['tokens'])
       elif todo=="move":
         self.client.move(arg)
       elif todo=="list_objects":