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 Reply #15 on Tue 15 Jun 2010 12:07 AM (UTC)
Message
Thanks Twisol.
I feel much better to hear timers and triggers are all single-threaded. Otherwise I have many place need to update in my scripts. :)

I actually notice triggers have similar evaluation order issue.
For instance, we have a trigger (TriggerA)with keep-evalution=true and sequence=100. In the script of the trigger, we create another trigger (TriggerB) with sequence 90. Then the TriggerA will be triggered twice.

In my personal understanding, for keep-evaluted trigger, the better algorithm is go throught all the trigger together, but don't execute the corresponding script, and only keep the trigger point in a list. When all matched triggers are found, we execute the script one by one. But anyway, it is not a big deal to me. :)
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #16 on Tue 15 Jun 2010 01:51 AM (UTC)
Message
Wanghom8228 said:
I actually notice triggers have similar evaluation order issue.
For instance, we have a trigger (TriggerA)with keep-evalution=true and sequence=100. In the script of the trigger, we create another trigger (TriggerB) with sequence 90. Then the TriggerA will be triggered twice.


That sounds odd. I've never run across that myself, though I admit I don't often create triggers dynamically.

Wanghom8228 said:
In my personal understanding, for keep-evaluted trigger, the better algorithm is go throught all the trigger together, but don't execute the corresponding script, and only keep the trigger point in a list. When all matched triggers are found, we execute the script one by one. But anyway, it is not a big deal to me. :)


Personally, I think that's a rather good point.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #17 on Tue 15 Jun 2010 02:05 AM (UTC)

Amended on Tue 15 Jun 2010 02:06 AM (UTC) by Twisol

Message
Huh, wow. Reproduced the above trigger issue, except it seems like it's gone into an infinite loop of some kind (and the program hangs). The trigger I used to test is below.


<triggers>
  <trigger
   enabled="y"
   keep_evaluating="y"
   match="foo"
   send_to="12"
   sequence="100"
  >
  <send>AddTriggerEx("", "foo", [[Note("Hi")]], 1, -1, 0, "", "", 12, 90)
Note("Trigger 1")</send>
  </trigger>
</triggers>


Tested with Simulate("foo\r\n").

When I attach to MUSHclient and pause it, it can't tell me where in the code it's paused at, which is annoying.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #18 on Tue 15 Jun 2010 02:19 AM (UTC)
Message
Reproduced again, minus the infinite loop.

<triggers>
  <trigger
   enabled="y"
   keep_evaluating="y"
   match="foo"
   send_to="12"
   sequence="100"
  >
  <send>if IsTrigger("testtrig") ~= 0 then
  AddTriggerEx("testtrig", "foo", [[Note("Hi")]], 1, -1, 0, "", "", 12, 90)
end
Note("Trigger 1")</send>
  </trigger>
</triggers>


Tested with: Simulate("foo\r\n")

Output:
foo
Trigger 1
Trigger 1


Something tells me the trigger list container uses an array offset as the position.
'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 #19 on Tue 15 Jun 2010 02:36 AM (UTC)
Message
Twisol said:

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


This definitely isn't STL here, so this isn't relevant, unfortunately.

In fact, as the MFC way of doing list and map iteration is to get the *next* pos while you are dealing with the current data (the data for the current pos) it is conceivable that if the current item invalidates the next item (which is the one pointed to by "pos") then it might indeed lead to a crash.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #20 on Tue 15 Jun 2010 02:38 AM (UTC)
Message
Nick Gammon said:
This definitely isn't STL here, so this isn't relevant, unfortunately.


Well, it's "relevant" - it's the same abstract structure, so it's not impossible for it to work the same way - but it's unlikely to be applicable.
'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 #21 on Wed 16 Jun 2010 02:34 AM (UTC)
Message
I can reproduce it with this:


require "addxml"
addxml.timer {  active_closed = true,
                second = 5,
                enabled = true,
                name = "timer1",
                send_to = sendto.script,
                send = [[

Note "In Timer1, Deleting timer2"
check (DeleteTimer "timer2")

]]
              }

addxml.timer {  active_closed = true,
                second = 5,
                enabled = true,
                name = "timer2",
                send_to = sendto.script,
                send = [[

Note "In Timer2, Deleting timer1"
check (DeleteTimer "timer1")

]]
              }


As I suspected, deleting an item in the list, one ahead of the one doing the deleting, invalidates the pointer (pos in the code) which should be pointing to the next item.

I have modified the timer handling to build a list of fired timers first, and then iterate through that list checking if the timer still exists. Thus, an item in the list, which an earlier timer deleted, won't be found, and the program won't crash.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #22 on Wed 16 Jun 2010 02:54 AM (UTC)
Message
It sounds like the same problem I found with triggers then, except I was adding and you were removing. Maybe the fix should be applied to triggers and aliases too.
'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 #23 on Wed 16 Jun 2010 03:58 AM (UTC)
Message
It is fiddlier in triggers - I looked.

A trigger that adds a trigger is kind-of a strange thing to do. I mean, it is like iterating through a table, from 1 to the end of the table, and for each item in the table, adding to it. This will create a loop, but is it really up to the table-handling code to detect this?
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #24 on Wed 16 Jun 2010 04:20 AM (UTC)
Message
Nick Gammon said:

It is fiddlier in triggers - I looked.

A trigger that adds a trigger is kind-of a strange thing to do. I mean, it is like iterating through a table, from 1 to the end of the table, and for each item in the table, adding to it. This will create a loop, but is it really up to the table-handling code to detect this?


Not detect it, no, but I imagine it should be able to compile a list of the matching triggers and run over them, similar to the timers, no? If the original list is subject to change, it seems like a sensible thing to do in general.
'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 #25 on Wed 16 Jun 2010 04:52 AM (UTC)
Message
Look, I don't want to spend a lot of time fixing a problem that hasn't even been reported in the wild. Your example earlier on was contrived based on the problems with timers being deleted.

Triggers are harder because at the point of matching you also have the matching start and end columns, you also have the issue of "repeat on same line", and the fact that a matching trigger stops further triggers matching.

The "correct" behaviour hasn't even been defined, and we may not even get agreement about that. Is it ...


  • To disallow adding triggers from with a trigger firing?

  • To allow it but process only the existing triggers at that point? (ie. to defer the adding of the new trigger, or the deleting of an existing one)

  • To reconstruct the trigger list, and take into account any additions and deletions, even during the traversal of the list?


I have amended the documentation for AddTrigger and similar functions to warn against adding a trigger while a trigger is firing.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #26 on Wed 16 Jun 2010 08:51 AM (UTC)

Amended on Wed 16 Jun 2010 09:01 AM (UTC) by Twisol

Message
Nick Gammon said:
Look, I don't want to spend a lot of time fixing a problem that hasn't even been reported in the wild. Your example earlier on was contrived based on the problems with timers being deleted.

Okay.



I'm unsure about the timer fix, can you explain this to me? You have a list 'firedTimersList', you're adding to it, and you get the start position, but then you use that position in 'm_strMapList'...


CStringList firedTimersList; // created
// ...
  firedTimersList.AddTail (strTimerName); // adding
// ...
POSITION pos = firedTimersList.GetStartPosition(); // start
// ...
  strTimerName = m_strMapList.GetNext (pos); // ???


EDIT: Also, it complains that there is no GetStartPosition. There is a GetHeadPosition though.
'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 #27 on Wed 16 Jun 2010 09:41 AM (UTC)

Amended on Wed 16 Jun 2010 10:15 AM (UTC) by Nick Gammon

Message
Very strange that it compiled, and even stranger that it ran OK.

Changed slightly now.

Copied and pasted a little too carelessly. ;)
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Twisol   USA  (2,257 posts)  Bio
Date Reply #28 on Wed 16 Jun 2010 03:41 PM (UTC)
Message
The GetStartPosition thing was a silly copy/paste error on my part as well (since I changed it from a for to a whole in my code)). You can disregard that!
'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,724 views.

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

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