[C++] Event Bugfix

[Lefloyd]

Hey,

German:

 Spoiler

dieser Bugfix ist vermutlich nur für wenige interessant, die sich schon etwas mehr mit dem Source auseinandergesetzt haben, aber diejenigen, die irgendwann mal dieses vermeintlich unlogische Problem bekommen sollten, welches ich heute gehabt habe und 3 Stunden an einer Lösung gesessen habe, denen möchte ich hiermit helfen.

Worum geht es in diesem Release? Ich möchte einen Fehler im Game-Source nennen, der Standartmäßig vorhanden ist - nur bisher nicht aufgefallen ist, da man diesen Fehler nur auf komplizierten Wegen erreicht. Der Fehler liegt in der Verbarbeitung von Events - also zeitabhängigen Ereignissen die einem bestimmten Intervall folgen. Der Fehler liegt darin, dass die Warteschlangenelemente vor dem Aufruf der Event-Funktion gelöscht werden (operator delete) und aber erst nach dem Aufruf der Pointer vom Event auf das Warteschlangenelement auf NULL gesetzt wird - sofern das Event während der Ausführung sich selbst beendet hat (durch "event_cancel") oder durch eine 0-Rückgabe der Event-Funktion.

Praktisches Beispiel für den Fehler (ich nehme jetzt mal den Weg, wie ich den Fehler bekommen habe):
a) In der Warteschlange von den Events wird das Warteschlangenelement W1 geladen
b) Dieses Element enthält Event E1 (welches einen Pointer auf W1 enthält)
c) Das Warteschlangenelement W1 wird gelöscht (operator delete)
d) Die Event-Funktion F1 wird von E1 aufgerufen
e) Diese Event-Funktion ruft eine weitere Funktion (F2) auf
f) In F2 wird ein neues Event (E2) erstellt
g) Dabei wird ein neues Warteschlangenelement W2 erstellt
h) Am Ende von F2 wird der Befehl "event_cancel(&E1)" aufgerufen (das Event (vermeintlich E1) wird beendet)
So, wo ist der Fehler? Der Fehler liegt darin, dass E2 niemals aufgerufen wird. Egal welche Zeit eingestellt wurde oder welche Funktion (F3) angegeben wurde, es wird niemals aufgerufen werden - da es von dem Befehl bei h) beendet wurde. Warum? Schauen wir es uns noch einmal detailierter mit Pointern an:

a) In der Warteschlange von den Events wird das Warteschlangenelement W1 geladen [W1->Data = E1]
b) Dieses Element enthält Event E1 [E1->pElement = W1]
c) Das Warteschlangenelement W1 wird gelöscht (operator delete) [E1->pElement = (gelöschtes) W1]
d) Die Event-Funktion F1 wird von E1 aufgerufen [E1->pElement = (gelöschtes) W1]
e) Diese Event-Funktion ruft eine weitere Funktion (F2) auf [E1->pElement = (gelöschtes) W1]
f) In F2 wird ein neues Event (E2) erstellt [W2->Data = E2; E2->pElement = W2; E1->pElement = (gelöschtes) W1 = W2]
g) Am Ende von F2 wird der Befehl "event_cancel(&E1)" aufgerufen [E1->pElement = (gelöschtes) W1 = W2 -> event_cancel(&E1) = event_cancel(&E2)]

Ich hoffe, dies war detailiert genug - für alle die sich mit kleinen Änderungen am Source auseinandersetzen ist dies sowieso noch uninteressant bzw. unrelevant.

Lösung für das Problem: ihr öffnet eure event.cpp & geht dort zur Funktion

int event_process(int pulse)

Dort scrollt ihr runter bis

new_time = (the_event->func) (get_pointer(the_event), processing_time);

und ändert den Teil

			new_time = (the_event->func) (get_pointer(the_event), processing_time);
			
			if (new_time <= 0 || the_event->is_force_to_end)
			{
				the_event->q_el = NULL;
			}
			else
			{
				the_event->q_el = cxx_q.Enqueue(the_event, new_time, pulse);
				the_event->is_processing = FALSE;
			}

zu diesem Teil ab:

			the_event->q_el = NULL;

			//sys_log(0, "EVENT: event %p info %p", &(*the_event), the_event->info);
			new_time = (the_event->func) (get_pointer(the_event), processing_time);
			
			if (new_time > 0 && !the_event->is_force_to_end)
			{
				the_event->q_el = cxx_q.Enqueue(the_event, new_time, pulse);
				the_event->is_processing = FALSE;
			}

English (not perfect ):

 Spoiler

This bugfix is only for these people interesting who are really working with the source. I did this for these who will maybe have this - alleged unlogical - problem in the future and that they won't have to search and search for a solution like I did it for three hours.. Well, let's begin.

What is this release about? I wanna name a mistake in the game source which is in every source (by ymir) - just nobody have found the mistake because you need to go complicated ways to produce it. The error lays in the processing of the events. The elements of the event queue are removed before the event is called (operator delete) but the pointer to them is removed after (removed = set to NULL) the event is called (it's only set to NULL if the event has stopped the repeat with a 0-return or with a call of "event_cancel").

A pratical way how to produce the error (I'll tell you the way I found it):
a) The event-queue loads / computes an element [Q1]
b) This element contains a pointer to an event [E1] (and the event [E1] contains a pointer to the queue element [Q1])
c) The queue element [Q1] will be removed (operator delete)
d) The function [F1] of the event [E1] will be called
e) This function [F1] calls another function [F2]
f) The function [F2] creates another event [E2] (which contains a pointer to the queue element [Q2] - also created with the new event)
g) At the end of [F2] the command "event_cancel(&[E1])" will be called (the event (supposed to be [E1]) will be canceled)
Well, where is the failure? The failure is: [E2] won't ever be called. You can insert every time and every function to the event [E2] but it won't ever be called - because it has been canceled at the command in g). Why? Let's have a more detailed look to this (with pointer this time):

a) The event-queue loads / computes an element [Q1] ([Q1]->Data = [E1])
b) This element contains a pointer to an event [E1] ([E1]->pElement = [Q1])
c) The queue element [Q1] will be removed ([E1]->pElement = (removed) [Q1])
d) The function [F1] of the event [E1] will be called ([E1]->pElement = (removed) [Q1])
e) This function [F1] calls another function [F2] ([E1]->pElement = (removed) [W1])
f) The function [F2] creates another event [E2] (and another queue element [Q2]) ([Q2]->Data = [E2]; [E2]->pElement = [Q2]; [E1]->pElement = (removed) [Q1] = [Q2]) -> [Q1] is removed so the address is free and if there is another event created just in the event function it will take exactly this address (tested it more than 10 times)
g) At the end of [F2] the command "event_cancel(&[E1])" will be called ([E1]->pElement = (removed) [Q1] = [Q2] -> event_cancel(&[E1]) = event_cancel(&[E2]))

I hope this was enough detailed - for everyone who doesn't really change the source it's not really interesting (or better irrelevant).

Solution for the problem: open your event.cpp & jump to the function

int event_process(int pulse)

Scroll down to the code

new_time = (the_event->func) (get_pointer(the_event), processing_time);

and change the part

			new_time = (the_event->func) (get_pointer(the_event), processing_time);
			
			if (new_time <= 0 || the_event->is_force_to_end)
			{
				the_event->q_el = NULL;
			}
			else
			{
				the_event->q_el = cxx_q.Enqueue(the_event, new_time, pulse);
				the_event->is_processing = FALSE;
			}

to the following part:

			the_event->q_el = NULL;

			//sys_log(0, "EVENT: event %p info %p", &(*the_event), the_event->info);
			new_time = (the_event->func) (get_pointer(the_event), processing_time);
			
			if (new_time > 0 && !the_event->is_force_to_end)
			{
				the_event->q_el = cxx_q.Enqueue(the_event, new_time, pulse);
				the_event->is_processing = FALSE;
			}

#UPDATE#:
I've written an example code to demonstrate you the bug in an easier way (at least for ppl who can program a bit).
Example result of the output WITHOUT my fix:
Example result of the output WITH my fix:
I added the 2 example files in the appendix - you just has to include the file "ExampleEventbug.h" in your cmd.cpp, write a function like this:

ACMD (do_call_example_script)
{
	CallExampleScript(ch);
}

And add the function into the command-list; just add the following code in the "cmd_info"-array:

{ "call_example_script",	do_call_example_script,	0,	POS_DEAD,	GM_IMPLEMENTOR },

Now you can go ingame (after compiling) and check out the output when you enter "/call_example_script" with and without my fix - you'll see same results as i've shown you in the screens. So without the 2. event won't be executed and with it will be.
#END OF UPDATE#

Vielleicht hilft es ja dem Einen oder Anderem

Kind Regards

[.Raicon]

Wie gewohnt, schlicht und einfach erklärt.

Danke für das Release.


LG .Raicon

[.BasH^#]

Danke ich war schon am verzweifeln

[LovecKrys]

And now in english.

[Lefloyd]

Added in english. If someone wanna write a better english translation - he may.

[Lefloyd]

#UPDATE#
I've got a request to give you an example for the bug - maybe I was a bit inaccurate in describing it. Well, I've added an example-file that you can compile and see how the fix works.

Kind Regards

[lollo_9_1]

Originally Posted by Lefloyd

#UPDATE#
I've got a request to give you an example for the bug - maybe I was a bit inaccurate in describing it. Well, I've added an example-file that you can compile and see how the fix works.

Kind Regards

I tried the code inside the ExampleEventbug.cpp file, but it worked fine without using your fix: (I just pasted it inside cmd.cpp)

It's a test server so, that character is the only one logged in. (I tried to spawn mobs/kill them/kill regen-spawned mobs/pull all/respawn all, but it didn't get bugs)

If we consider your two screenshots (1. 2. ) we can easily figure out that event1 and event2 are pointing to the same queueElementPtr. So, when you call event_cancel(&f_pkEvent1); you are instead calling event_cancel(&f_pkEvent2);, due to an incrementing issue that moves f_pkEvent1 to f_pkEvent1"-1" and f_pkEvent2 (f_pkEvent1"+1" in theory) to f_pkEvent1. (f_pkEvent1->q_el-1 to be more precise) (is this what you wrote above in the first post?)
Why? When you call f_pkEvent2 = event_create(example_event2, info, PASSES_PER_SEC(3)); you're adding a new event in the event list using the function, which adds the event at the end of the list if it doesn't find a clone (or a greater one, but I don't think this is the case), which means you already have this event in your event list and you, but only you, got this bug due to a weird implementation you made.
If other people got this bug using not-so-modded-source-files, maybe the trigger of such issue is to refer to something else. I haven't got it yet.
I don't think a trivial the_event->q_el = NULL; could make the difference in there. (it's just a pointer, otherwise we would have thought about an overloaded assignment operator fault)

Speaking of which, is dominion_manager.cpp part of the files to test or did you put it in the .rar instead of ExampleEventbug.h?

[Lefloyd]

Thanks for your reply lollo_9_1, that what you explained was that what I tried to explain. I also thought when I found out the source of the bug that the bug would not be there ANY time because some times the address will be refered by some other things before the new create_event is called, but in my files it occurs when I don't add my fix. And yes, this NULL set will fix the bug (when it would occur) because the event_cancel set's the "bCancel" variable in the queueElement that is refered through "q_el" from the eventPtr. If the eventPtr has the reference of the queueElement of another event (the new event) it will cancel it - if there is a NULL pointer it will do nothing. That's why this fix works. And yes, it won't occur everytime but it CAN occur - I know this for sure because it does occur at my files if I haven't changed the position of the NULL pointer setting..

I hope you understand what I'm trying to say, I knew why the error came (I explained it in my first post) I just send the pictures to show you what the results can be like.

€dit: And no, the dominion_manager wasn't a part of the example, sorry for uploading it.^^ I've already deleted it in the rar-file.
€dit2: And why it doesn't work at you (the error) is because your queueElementPtr hadn't be the same - maybe it's also different from system to system if the bug occurs. But there can be the case, that it occurs.^^
€dit3: Aaand without my fix I'd wonder if he wouldn't set the bCancel of a address which is already destroyed... and I can't believe that this is good

Kind Regards

[.iRyan]

Thanks for this release!