Tracking down a teleport bug...

[supremoleet]

Another fun one... heh. While tinkering with our server, one of my GMs added a moongate and setscptrig 5011 on it. I went through it, selected my destination and it worked fine... however... it left behind a clone of myself at the gate. When the automatic worldsave kicked in, it seg faulted the server due to an invalid pointer.

Code: Select all

(gdb) #0  0x00002b2737e917c9 in raise () from /lib/libc.so.6
#1  0x00002b2737e92bae in abort () from /lib/libc.so.6
#2  0x00002b2737ec63be in __fsetlocking () from /lib/libc.so.6
#3  0x00002b2737ecb79c in malloc_usable_size () from /lib/libc.so.6
#4  0x00000000006b3a54 in js_PurgeDeflatedStringCache ()
#5  0x00000000006b3ac7 in js_FinalizeStringRT ()
#6  0x00000000006b3a82 in js_FinalizeString ()
#7  0x000000000067eeb3 in js_GC ()
#8  0x000000000067e837 in js_ForceGC ()
#9  0x000000000065da4a in JS_GC ()
#10 0x000000000047c025 in UOX::CJSEngine::CollectGarbage ()
#11 0x000000000061984a in UOX::CollectGarbage ()
#12 0x0000000000653610 in UOX::CWorldMain::SaveNewWorld ()
#13 0x000000000061d880 in UOX::CWorldMain::CheckAutoTimers ()
#14 0x000000000061f4a0 in main ()

After lots of crashing and testing, we determined the problem seems to be that the duplicate instance of the player using the moongate is causing the save to flip out and die.

First place I went to is onGumpPress in ./js/items/moongate.js. There, I found where the actual teleport is called in the script. It's using srcChar.Teleport(X, Y, Z, World) to make the move. I tried to track this down in the source and think I found it in cChar.cpp, CChar::Teleport(void).

The curious thing is that I see the "go" and "tele" commands use SetLocation instead -- is there a reason for this? I suppose I could potentially change Teleport to SetLocation as a workaround, but ideally I'd like to try to figure out the root of the problem with the Teleport function and correct it (especially since I'm not sure if anything else uses it at the moment).

Any ideas what would be causing the character to leave behind a clone of it self?

[supremoleet]

That didn't work so well -- I made the change to SetLocation, it still is doing the same thing. Tried it several times, it still leaves a copy of the character at the gate, and the next time a save occurs (whether it's manual or automatic), the server crashes.

Some excerpts from the onGumpPress function...

Code: Select all

  //Calc char
  var srcChar = srcSock.currentChar;
  ...
  switch(myButton)
  {
    //Tram Fell (world 0 and 1)
    case 5:
    case 13:
    {
      srcChar.SetLocation( 1336,1998,5,tWorld );    //Brit
      break;
    }
    ...
  }
  ...
} 

Previously it was the same thing, just substitute SetLocation with Teleport. It takes me to the correct location (although after logging out or crashing, if it's not on Felucca the character logs in underground but that's another issue I suppose) but crashes on world save.

Next what I did was edit the onCollide part of moongate.js to:

Code: Select all

function onCollide( srcSock, pUser, iUsed ) 
{ 
	// For testing purposes:
	var srcChar = srcSock.currentChar;
	srcChar.Teleport( 1336,1998,5,0 );    //Brit 
}

It works fine -- no duplicate generated. So now I'm wondering if it has to do with the gump...

Code: Select all

function onGumpPress(srcSock,myButton) 
{ 
  //Calc char 
  var srcChar = srcSock.currentChar; 

  //Range check. Its possible to call up the gate and run away 
  //get the items's serial 
  var oldSerial = new Array; 
  var iCheck = new String(srcChar.GetTag("ItemUsed")); 
  var oldSerial = iCheck.split(','); 
  //Convert serial into actual numbers not strings 
  oldSerial[0] = parseInt(oldSerial[0]) 
  oldSerial[1] = parseInt(oldSerial[1]) 
  oldSerial[2] = parseInt(oldSerial[2]) 
  oldSerial[3] = parseInt(oldSerial[3]) 
  //Find out what the item is from the serial 
  var iUsed=CalcItemFromSer(oldSerial[0], oldSerial[1], oldSerial[2], oldSerial[3]); 
  ret=iUsed.InRange( srcChar, 5); 

  //Clear the tag so we can use gate again? 
  srcChar.SetTag("ItemUsed", "0,0,0,0");  

  //Check ret and button (don't say anything if canceled) 
  if( ret == false && myButton >= 0) 
  { 
    srcSock.SysMessage("You have moved too far away to use this."); 
    return 
  } 

  //Where we going... Fel or tram 
  if(myButton >=5 && myButton <12>=13 && myButton <= 20) 
    tWorld=1;  //Going to fel 

  switch(myButton) 
  { 
      ...
  }

}

[Maarc]

I'll try to give a little insight here, as best as I can ...

Teleport() and SetLocation() map to the same function call underneath, actually. They are completely interchangable. I think at one time they did do different things, but rather than break legacy scripts, they ended up mapping to the same function.

With regards to the crash ... that's an oddity! I think I understand what you are saying about leaving a duplicate behind, but I don't think it really IS a duplicate ... I suspect we're not sending the right packets in the right order for map changes, and consequently, clients aren't being told to 'forget' properly.

A way to test this would be with 3 clients: A uses the moongate to disappear, B is standing beside the moongate, and C wanders into that region (from a reasonable distance!) well AFTER A has used the moongate. My suspicion is that B will still see A, but C will have no clue they were ever there.

But hmmm, that crash is a truly odd one ... sounds like the JS engine is failing to release some of the memory that it has allocated to it somehow. Which is likely caused by something we are doing, rather than anything they are ...

As a suggestion to try and eliminate us, see if you can't update to the latest CVS code for spidermonkey, link against that, and see if it works ... big ask, I know! But if it works, then we can knock it down to a bug in their implementation. If it doesn't ... we're being very naughty. And I'm not sure just by looking at it how we are: I suspect we use arrays and strings like that in other places as well.