- changed milestone to 2.0
-
assigned issue to
Access violation in TEvent.InvokeEventHandlerStub/InternalInvoke when adding a eventhandler while in an eventhandler
- the problem first appeared with commit event optimizations
- still persists with current commit 070b8ae (the line numbers in the text refer to this state)
In our main project the symptom is an access violation read at 0x00000000 at line 466 of Spring.Events, which is “pop edx” in TEvent.InvokeEventHandlerStub (that's what the IDE shows). I could pinpoint the actual problem to line 675 of TEvent.InternalInvoke at which handlers.Code is nil.
In my attempts to replicate the issue in a smaller project I could at least get a read access violation at 0x80808080 (freed memory) with both handlers.Code and .Data being 0x80808080.
The prerequisites seem to be quite complex with an event having multiple handlers, which have to access the container (?) and adding another handler in a sub-thread.
Note that the code does leak one instance of TC, which is expected.
program Project41;
uses
FastMM4,
System.Classes,
Spring,
Spring.Container;
type
TEvent = procedure() of object;
type
TC = class
public class var
Event: Event<TEvent>;
public
class procedure EventHandler1();
class procedure EventHandler2();
procedure EventHandler3();
procedure EventHandler4();
end;
class procedure TC.EventHandler1();
begin
GlobalContainer.Resolve<TC>().Free;
end;
class procedure TC.EventHandler2();
begin
TThread.CreateAnonymousThread(
procedure
begin
var x := TC.Create;
TC.Event.Add(x.EventHandler4);
end).Start;
TThread.Sleep(1000);
end;
procedure TC.EventHandler3();
begin
//dummy
end;
procedure TC.EventHandler4();
begin
//dummy
end;
var c: TC;
begin
GlobalContainer.RegisterType<TC>;
GlobalContainer.Build;
c := TC.Create;
c.Event.Add(c.EventHandler1);
c.Event.Add(c.EventHandler2);
c.Event.Add(c.EventHandler3);
TC.Event.Invoke();
c.Free;
end.
Comments (14)
-
repo owner -
repo owner - changed status to open
-
repo owner - changed status to resolved
fixed
#361→ <<cset 3687a86a0825>>
-
reporter - changed status to open
While the MCVE does run without issues with the latest changes I can still replicate the issue in our main project. The only difference being that the address changed (from access to 0x00000000 to some random location).
I was not able to create a new MCVE yet, just wanted to let you know that this issue doesn't seem to be fixed.
-
reporter - attached Spring.HazardEra_Withlogging.pas
Even adding logging to all the public functions of Spring.HazardEra does alter the execution enough that the problem disappears. So it seems to be a race condition. I’m currently out of ideas on how to recreate the issue. Any ideas on your side?
-
repo owner Is the situation when the AV occurs still the same? I mean still during Invoke and some other thread modifies the invoked event handler list or at other times?
Update: I managed to produce a stress test scenario that involves invoking and adding/removing event handlers which causes a leak that might as well be the cause for an AV in your scenario. Will investigate.
-
repo owner Found and fixed the issue I mentioned - while I could only reliably reproduce a memory leak caused by this I suspect this could also have been the cause for AVs.
Please let me know if this works now for you.
-
reporter - attached Project41.dpr
The latest change doesn't seem to help. Everything still the same as I descripted initially. On the plus side turning my example into a "stress test" does result in an access violation even with the most recent changes.
-
repo owner Well then I need some code with this behavior to analyze further.
-
reporter That’s why i attached the code “Project41.dpr”
-
repo owner Sorry, I missed that
Thanks, will look into
-
repo owner Next attempt - this should finally do it
The issue was that the internal control blocks were reused and overridden when a guard was acquired while the current thread already had a guard acquired (during Invoke). This caused the prior guarded pointer to become unguarded and was potentially freed and being reused by the memory manager causing those AVs. Every guard now uses its own control block.
Please keep in mind that your latest project might still raise an AV which is being caused by the application already terminating while the anonymous threads are still running.
Thanks again for testing and being patient.
-
reporter That fixed it! Thanks.
-
repo owner - changed status to resolved
fixed
#361→ <<cset a7d543927ceb>>
- Log in to comment
And I almost thought it’s ready for release
I’ll look into it later. Thanks for testing though! Much appreciated.