Register forum user name Search FAQ

Gammon Forum

Notice: Any messages purporting to come from this site telling you that your password has expired, or that you need to verify your details, confirm your email, resolve issues, making threats, or asking for money, are spam. We do not email users with any such messages. If you have lost your password you can obtain a new one by using the password reset link.

Due to spam on this forum, all posts now need moderator approval.

 Entire forum ➜ MUSHclient ➜ Bug reports ➜ Crash Bug: DeleteTimer crash the MUSHClient
Crash Bug: DeleteTimer crash the MUSHClient

It is now over 60 days since the last post. This thread is closed.     Refresh page

Pages: 1 2  

Posted by Wanghom8228   (19 posts)  Bio
Date Mon 14 Jun 2010 02:02 AM (UTC)

Amended on Mon 14 Jun 2010 07:33 AM (UTC) by Wanghom8228

Message
Repro Step at MUSHClient 4.51:
1. Create a MCL with following alias & timer.

<aliases>
 <alias match="CT" enabled="y" send_to="12" sequence="100">
  <send>world.EnableTimer("tm_delete")
local name = "tm_test"
local seconds = 2
--local script = "test()"
  check (world.AddTimer (
      name, 
      0, 
      0, 
      seconds, 
      "",
      timer_flag.Enabled + timer_flag.OneShot + timer_flag.Temporary + timer_flag.Replace, 
      ""))  
</send>
  </alias>
</aliases>

<timers>
  <timer name="tm_delete" second="1.00" offset_second="0.00" send_to="12">
  <send>world.Note("timer")
world.DeleteTimer("tm_test")
--world.EnableTimer("tm_test", false)</send>
  </timer>
</timers>



2. Open the MCL and connect to the MUD game.
3. Type alias "CT".
4. The MUSHClient will crash.

It not always repro, but very easy. You might need to do the #3 quickly after connect to the game.
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #1 on Mon 14 Jun 2010 02:06 AM (UTC)
Message
I don't see an alias TC (though you did post a CT, did you mean that?), and I don't see DeleteTimer() used anywhere at all.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #2 on Mon 14 Jun 2010 04:47 AM (UTC)
Message
I can't reproduce this. Did you copy and paste the aliases directly?

Template:copying For advice on how to copy aliases, timers or triggers from within MUSHclient, and paste them into a forum message, please see Copying XML.


I don't see this variable used anywhere:


local script = "test()"


The timer you add (called tm_test) doesn't seem to do anything.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Wanghom8228   (19 posts)  Bio
Date Reply #3 on Mon 14 Jun 2010 07:15 AM (UTC)
Message
My bad. I had some typo in the script. I have fixed it.
The crash happen in the DeleteTimer, but if i change the DeleteTimer to EnableTimer(false), it works fine.
It is my only workaround now.
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #4 on Mon 14 Jun 2010 07:16 AM (UTC)
Message
DeleteTimer will delete a timer completely. EnableTimer("name", false) simply disables it, so it still exists but is inactive.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Wanghom8228   (19 posts)  Bio
Date Reply #5 on Mon 14 Jun 2010 07:16 AM (UTC)
Message
It doesn't matter where is "test()" function, since it is never expected to be executed. I just want to show a min repro to you guys.
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #6 on Mon 14 Jun 2010 11:24 AM (UTC)
Message
So in effect you are saying that if one timer deletes another, it crashes, is that it?
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Wanghom8228   (19 posts)  Bio
Date Reply #7 on Mon 14 Jun 2010 12:34 PM (UTC)

Amended on Mon 14 Jun 2010 12:36 PM (UTC) by Wanghom8228

Message
Yes. I think so.
BTW: Nick, can you repro it in your box? Is it a real bug? Or caused by my bad usage?
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #8 on Mon 14 Jun 2010 06:43 PM (UTC)
Message
I tried setting up two timers and having one delete the other. No crash here. I'm using a custom build of 4.52,, but I haven't really changed any functionality.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #9 on Mon 14 Jun 2010 09:53 PM (UTC)
Message
There is a test in the timer code that you cannot delete a timer (delete itself) whilst a script is running. This is probably because deleting something while it is executing is a bad idea (the pointer to the timer script becomes invalid while the script is running).

Although you are not doing that, it is possible that a timer that deletes another timer could cause a crash, as it may modify the list of timers that it is currently traversing (in other words, you may remove the item from the list that is about to be tested).

You may also defeat the internal test by having one timer call a script which causes a different timer to delete the first one (somehow, I can't quite get my head around that one).

You could explore using EnableTimer (timername, false) to simply disable it, or use one-shot timers which self-delete at a safe time (although even that looks slighly dubious, looking at the code).

Twisol, in doc.cpp around line 4448, is the code that goes through timers:


  for (POSITION pos = TimerMap.GetStartPosition(); pos; )
    {
    TimerMap.GetNextAssoc (pos, strTimerName, timer_item);


I'm not sure how reliable the GetNextAssoc is, if the map changes (ie. a timer is deleted elsewhere, like in a script). Effectively, pos could become invalid, as far as I can see.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #10 on Mon 14 Jun 2010 09:53 PM (UTC)
Message
Wanghom8228 said:

Yes. I think so.
BTW: Nick, can you repro it in your box? Is it a real bug? Or caused by my bad usage?


I haven't reproduced it yet.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #11 on Mon 14 Jun 2010 11:16 PM (UTC)
Message
Nick Gammon said:
Twisol, in doc.cpp around line 4448, is the code that goes through timers:


  for (POSITION pos = TimerMap.GetStartPosition(); pos; )
    {
    TimerMap.GetNextAssoc (pos, strTimerName, timer_item);


I'm not sure how reliable the GetNextAssoc is, if the map changes (ie. a timer is deleted elsewhere, like in a script). Effectively, pos could become invalid, as far as I can see.


That's an interesting point. I don't know what CTypedPtrMap (or CMapStringToPtr I suppose) uses internally, but if it uses a linked list it should be immune (as long as you don't remove the current one).

Hrm. While timers just have a single map definition (CTimerMap), aliases and triggers seem to have array and list typedefs as well. I understand that the array is used for sequencing, which timers don't need, but then there's the list. I assume the list exists precisely to avoid these issues, because as long as you don't remove the current item, it's safe. Array iterators will be invalidated if you remove anything before the current position, in the best case.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Wanghom8228   (19 posts)  Bio
Date Reply #12 on Mon 14 Jun 2010 11:18 PM (UTC)

Amended on Mon 14 Jun 2010 11:40 PM (UTC) by Wanghom8228

Message
Nick Gammon said:

There is a test in the timer code that you cannot delete a timer (delete itself) whilst a script is running. This is probably because deleting something while it is executing is a bad idea (the pointer to the timer script becomes invalid while the script is running).

Sounds like the each timers is executing in a independent thread? Now I am worry about the scenario of editing the same variable in different timeer, for example:


-- variable rooms is a global variable in the script

-- timer function is 
function OnTimer(newroom)
  local temp = rooms
  temp = rooms.."|"..newroom
  rooms = temp
end

if the OnTimer() function is invoked in different timer, will it lead me to some incorrect result? for example one of the newroom is not be added.

Nick Gammon said:

Twisol, in doc.cpp around line 4448, is the code that goes through timers:

I know MUSHClient is free. Is it open source? Where I can find the source code?

Nick Gammon said:

I haven't reproduced it yet.

Just try to create those two alias and timer in your game. Re-open and connect to game, type "CT". My MUSHClient is 4.51.
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #13 on Mon 14 Jun 2010 11:25 PM (UTC)
Message
Wanghom8228 said:

Nick Gammon said:

There is a test in the timer code that you cannot delete a timer (delete itself) whilst a script is running. This is probably because deleting something while it is executing is a bad idea (the pointer to the timer script becomes invalid while the script is running).

Sounds like the each timers is executing in a independent thread? Now I am worry about the scenario of editing the same variable in different timeer, for example:


-- variable rooms is a global variable in the script

-- timer function is 
function OnTimer(newroom)
  local temp = rooms
  temp = rooms.."|"..newroom
  rooms = temp
end

if the OnTimer() function is invoked in different timer, will it lead me to some incorrect result? for example one of the newroom is not be added.


No, it's practically all single-threaded. The issue is that we could be running over the list of timers to fire the ones that need to go off, and one of those timers might modify the list we're going over. It's like you're on a set of train tracks, and suddenly a section of the track disappears. When the timer finishes and we continue checking the list, we could be in undefined territory. It depends on how the list is implemented internally. If it's an array, and the item that was removed (or added) is in an earlier part of the array, that's bad. If it's a linked list, it's perfectly fine as long as we didn't remove the one that we're on right now.


Wanghom8228 said:
Nick Gammon said:

Twisol, in doc.cpp around line 4448, is the code that goes through timers:

I know MUSHClient is free. Is it open source? Where I can find the source code?


At GitHub [1]. I have a fork I'm tinkering with too, but Nick's is the official one.

[1] http://github.com/nickgammon/mushclient
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #14 on Mon 14 Jun 2010 11:28 PM (UTC)
Message
This page [1] implies says that map iterators should remain valid if the map changes underneath it, but I don't know if it applies to the MFC containers.

[1] http://www.sgi.com/tech/stl/Map.html
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.

90,725 views.

This is page 1, subject is 2 pages long: 1 2  [Next page]

It is now over 60 days since the last post. This thread is closed.     Refresh page

Go to topic:           Search the forum

[Go to top] top

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

Home