Project JEDI - Issue Tracker
Mantis Bugtracker

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0005026 [JEDI VCL] 00 JVCL Components major random 2009-11-19 15:07 2012-09-10 14:15
Reporter ujr View Status public  
Assigned To AHUser
Priority normal Resolution fixed  
Status resolved   Product Version Daily / GIT
Summary 0005026: JvScheduledEvents - race condition
Description Hi -
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;
-------
Tags No tags attached.
Attached Files zip file icon SchedRace.zip [^] (7,191 bytes) 2009-12-07 13:24

- Relationships

-  Notes
(0016942)
obones (administrator)
2009-12-04 15:27

Please send us the zipped sources of a sample application showing this
(0016973)
ujr (reporter)
2009-12-07 13:28

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.
(0016974)
ujr (reporter)
2009-12-07 13:30

(btw, can I change the state of the report or is this done by maintainers?)
(0016975)
obones (administrator)
2009-12-07 15:36

Thanks for the sources, that should be enough.
And please let us handle the statuses, they have meaning to us
(0019901)
obones (administrator)
2012-06-12 11:23

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
(0019909)
ujr (reporter)
2012-06-12 13:12

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.
(0019937)
obones (administrator)
2012-06-12 17:46

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.
(0020001)
AHUser (developer)
2012-06-15 10:26

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 09:25 AHUser Note Added: 0020000
2012-06-15 10:24 AHUser Note Deleted: 0020000
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


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker