View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005026JEDI VCL00 JVCL Componentspublic2009-11-19 15:072012-09-10 14:15
ReporterujrAssigned ToAHUser 
PrioritynormalSeveritymajorReproducibilityrandom
Status resolvedResolutionfixed 
Product VersionDaily / GIT 
Target VersionFixed in Version3.46 
Summary0005026: JvScheduledEvents - race condition
DescriptionHi -
there's a race condition in JvScheduledEvents.pas related to how execution of TJvCustomScheduledEvents is handled:
TScheduleThread delegates execution by PostMessage to the GUI thread. This is done for *all* events/schedules that are due one by one.
The GUI thread gets the pointer to the event from PostMessage params and calls
 DoStartEvent(TJvEventCollectionItem(WParam));
 TJvEventCollectionItem(WParam).Execute;
 DoEndEvent(TJvEventCollectionItem(WParam));
also one by one.

If for any reason (it does not seem to be a rare case, IMO), one or all of the schedules are deleted during TJvEventCollectionItem(WParam).Execute, the next use of a WParam (for the active schedule in DoEndEvent or waiting in the message queue for the next Dispatch(Msg)) results in an access violation.

My (quick) fix was to create a copy of the event TskColl[I] in TScheduleThread.Execute, which is handled to PostMessage. Such, because of the critical section, all "original" events should be treated fine. After DoEndEvent(TJvEventCollectionItem(WParam)) the copy of the event will be freed so there's no memory leak.

Unfortunately, it does not seem to be the perfect solution (although it works for me). The problem is, that only during Execute of TJvEventCollectionItem, the NextScheduledFire timestamp is determined. Because this is done for the copy, the scheduled event is only released once.
Because of this, there's an additional hack necessary: Execute the original schedule without calling the actual application handler. Sure, there's a (short) time of inconsistency which, however, doesn't matter in my project.

Below you'll find my (probably quick 'n dirty) fixes - however, since I don't know enough about the Jcl/Jvcl/JvScheduledEvents interna you'll certainly come up with a better fix.

Thanks
 
Additional Information-------
procedure TScheduleThread.Execute;
var
  TskColl: TJvEventCollection;
  I: Integer;
  SysTime: TSystemTime;
  NowStamp: TTimeStamp;
  { +++ }
  Event: TJvEventCollectionItem;
  DummyCollection: TJvEventCollection;
  { ### }
begin
  try
    FEnded := False;
    { +++ }
    DummyCollection:=TJvEventCollection.Create(nil);
    { ### }
    while not Terminated do
    begin
      if (FCritSect <> nil) and (FEventComponents <> nil) then
      begin
        FCritSect.Enter;
        try
          FEventIdx := FEventComponents.Count - 1;
          while (FEventIdx > -1) and not Terminated do
          begin
            GetLocalTime(SysTime);
            NowStamp := DateTimeToTimeStamp(Now);
            with SysTime do
              NowStamp.Time := wHour * 3600000 + wMinute * 60000 + wSecond * 1000 + wMilliseconds;
            TskColl := TJvCustomScheduledEvents(FEventComponents[FEventIdx]).Events;
            I := 0;
            while (I < TskColl.Count) and not Terminated do
            begin
              if (TskColl[I].State = sesWaiting) and
                (CompareTimeStamps(NowStamp, TskColl[I].NextFire) >= 0) then
              begin
                TskColl[I].Triggered;
                { +++ }
                Event:=TJvEventCollectionItem.Create(DummyCollection);
                Event.Start;
                Event.Data:=TskColl[I].Data;
                Event.CountMissedEvents:=TskColl[I].CountMissedEvents;
                Event.Name:=TskColl[I].Name;
                Event.Schedule:=TskColl[I].Schedule;
                Event.OnExecute:=TskColl[I].OnExecute;
                Event.Triggered;
                { simulate execution of event }
                TskColl[I].OnExecute:=nil;
                TskColl[I].Execute;
                TskColl[I].OnExecute:=Event.OnExecute;
                { inconsistency from this moment on until the copy is executed too }
                  { ### }
                PostMessage(TJvCustomScheduledEvents(FEventComponents[FEventIdx]).Handle,
                  CM_EXECEVENT, Integer(Event), 0); { +++ instead of Integer(TskColl[I]) ### }
              end;
              Inc(I);
            end;
            Dec(FEventIdx);
          end;
        finally
          FCritSect.Leave;
        end;
      end;
      if not Terminated then
        Sleep(1);
    end;
  except
  end;
  { +++ }
  FreeAndNil(DummyCollection);
  { ### }
  FEnded := True;
end;
-------
procedure TJvCustomScheduledEvents.CMExecEvent(var Msg: TMessage);
begin
  with Msg do
    try
      DoStartEvent(TJvEventCollectionItem(WParam));
      TJvEventCollectionItem(WParam).Execute;
      DoEndEvent(TJvEventCollectionItem(WParam));
      { +++ }
      try
        ScheduleThread.Lock;
        FreeAndNil(TJvEventCollectionItem(WParam));
      finally
        ScheduleThread.Unlock;
      end;
      { ### }
      Result := 1;
    except
      if Assigned(ApplicationHandleException) then
        ApplicationHandleException(Self);
    end
end;
-------
TagsNo tags attached.

Activities

obones

2009-12-04 15:27

administrator   ~0016942

Please send us the zipped sources of a sample application showing this

2009-12-07 13:24

 

SchedRace.zip (7,191 bytes)

ujr

2009-12-07 13:28

reporter   ~0016973

Hi -

please see "SchedRace.zip".
I could provide a pre-compiled debug version, if you need it.

You may want to use FastMM debug versions to get more information, but the default memory manager will show the access violation as well.

ujr

2009-12-07 13:30

reporter   ~0016974

(btw, can I change the state of the report or is this done by maintainers?)

obones

2009-12-07 15:36

administrator   ~0016975

Thanks for the sources, that should be enough.
And please let us handle the statuses, they have meaning to us

obones

2012-06-12 11:23

administrator   ~0019901

I'm sorry, this is a C5 project and we no longer support C5.
Please update your BCB installation, the JCL and JVCL one then let us know if this is still here

ujr

2012-06-12 13:12

reporter   ~0019909

I'm sorry, but this must be a joke.

It takes 2 1/2 year only to tell now you don't support the project files anymore? Does this make the bug invalid?

I pointed out where I see the bug - the whole procedure with changes that fixed it for me. I sent you a project file that you requested for your convenience. And which you accepted a long long time ago.

And now I should update my BCB installation and confirm it again? No, sorry, if you don't really care, I don't either. So go ahead and close the bug.

It's a pity about the time it took us.

obones

2012-06-12 17:46

administrator   ~0019937

Well, I understand that you are upset by the current situation, but even two years ago, there were not very many users C++ Builder 5.
I agree that you have provided a good report with detailed instructions, but considering that the code has evolved during those two years, I would like to reproduce the issue before making any change.
Especially because the required change might be a bit different from your proposal, as you even have doubts about it.
What you have to understand is that I'm pretty much alone in this ship and so I'm only looking at issues when I have spare time, which goes rarer by the day.
And when I have time to look at issues, I prioritize them by potential number of users and complexity, which means that C5/C6 issues are very low in the list.

I'm sorry about the situation, but with the limited resources I have at hand, that's the only thing that I can provide.

AHUser

2012-06-15 10:26

developer   ~0020001

This is fixed in svn revision 13356.

Changes:
The triggered events are put into a list that is updated if a event is deleted. There is only one message posted to the message queue that will handle all posted events and the events that are posted while the message handler is executing the events.

Issue History

Date Modified Username Field Change
2009-11-19 15:07 ujr New Issue
2009-12-04 15:27 obones Note Added: 0016942
2009-12-04 15:27 obones Status new => feedback
2009-12-07 13:24 ujr File Added: SchedRace.zip
2009-12-07 13:28 ujr Note Added: 0016973
2009-12-07 13:30 ujr Note Added: 0016974
2009-12-07 15:36 obones Note Added: 0016975
2009-12-07 15:36 obones Status feedback => acknowledged
2012-06-12 11:23 obones Note Added: 0019901
2012-06-12 11:23 obones Status acknowledged => feedback
2012-06-12 13:12 ujr Note Added: 0019909
2012-06-12 17:46 obones Note Added: 0019937
2012-06-15 10:26 AHUser Note Added: 0020001
2012-06-15 10:26 AHUser Status feedback => resolved
2012-06-15 10:26 AHUser Fixed in Version => Daily / SVN
2012-06-15 10:26 AHUser Resolution open => fixed
2012-06-15 10:26 AHUser Assigned To => AHUser
2012-09-10 14:15 obones Fixed in Version Daily / SVN => 3.46