Icon View Thread

The following is the text of the current message along with any replies.
Messages 1 to 8 of 8 total
Thread Threading problems...
Wed, May 23 2007 7:01 AMPermanent Link

=?iso-8859-1?Q?Thomas_Eg_J=F8rgensen?=
Hi,

I've been using DBISAM for many years, and i thought that i was smart
enough to develop a "fullblown multithreading database application"
using DBISAM 3...guess notSmile

My problem is that my threads occasionally fails with an Access
violation at some "random" address. It always happens when the thread is
trying to free the session at destruction. It only happens in <1% of the
cases, but it happens! I have around 50 users which uses this program
many hours a day(freeing a thread every minut or so) and i've recorded a
couple of hundred incidents the last couple of weeks...I've tried to
follow the steps here during the design:
http://www.elevatesoft.com/dbisam4d5_multi_threaded_applications.htm

The threads always use a database-class i've created. Every thread uses
TDBISAMQuery-components for databasemanipulation. On Creation the
threads query is connected to it's own database-class, e.g.:
******** code ********
constructor TMyThread.Create(Settings: TNPSettings);
begin
 inherited create(True);

 Priority:=tpLower;

 //Initialize database object
 FDatabase:=TNPDatabase.Create;
 FDatabase.Initialize(settings.DatabasePath);

 //initialize mainquery
 FQuery:=TDBISAMQuery.Create(nil);
 FQuery.DatabaseName:=FDatabase.DatabaseName;
 FQuery.SessionName:=FDatabase.SessionName;

 FQuery.SQL.Clear;
 FQuery.SQL.Add('SELECT SOME STUFF');
 FQuery.Prepare;

end;

destructor TMyThread.Destroy;
begin
 FQuery.Close;
 FQuery.UnPrepare;
 FQuery.free;

 FDatabase.free;

 inherited destroy;
end;

******** code ********

Most of the source-code for the database-class is included here:
******** code ********
type
 TNPDatabase = class
 private
   FDatabase : TDBISAMDatabase;
   FSession  : TDBISAMSession;
   FQuery    : TDBISAMQuery;

   FInitialized: boolean;

   function GetDatabaseName:string;
   function GetSessionName:string;
 public
   property DatabaseName  : string read GetDatabaseName;
   property SessionName   : string read GetSessionName;

   constructor Create;
   destructor Destroy; override;

   procedure Initialize(DBLocation: string);
 end;

implementation

constructor TNPDatabase.Create;
begin
 //Allways call inherited Create first!
 inherited Create;

 FInitialized:=false;

 //Create nessesary objects
 FDatabase:=TDBISAMDatabase.create(nil);
 FSession:=TDBISAMSession.create(nil);
 FQuery:=TDBISAMQuery.create(nil);
end;

Destructor TNPDatabase.Destroy;
begin
 //Write destroy procedures
 FQuery.Free;
 FSession.Free;    <---//ERROR: Access Violation here!
 FDatabase.Free;

 //Allways call inherited destroy last!
 inherited Destroy;
end;

procedure TNPDatabase.Initialize(DBLocation: string);
begin
 //Check wheter or not the nessary objects a created
 if (FSession<>nil) and (FDatabase<>nil) then
 begin
   //Allways use auto session names
   FSession.AutoSessionName:=true;

   //Set potential remote information
   FSession.RemoteHost:=DBLocation;
   FSession.RemoteUser:='';
   FSession.RemotePassword:='';

   //Set potential local information
   FDatabase.Directory:=DBLocation;
   FDatabase.DatabaseName:='MyDatabase'+IntToStr(RandomRange(0,1000000));

   //Set connectiontype(remote/local):
   FSession.SessionType:=stLocal;

   //Link the session and database together
   FDatabase.SessionName:=Fsession.SessionName;

   FQuery.DatabaseName:=GetDatabaseName;
   FQuery.SessionName:=GetSessionName;

   //Use temporary folder for temp files!
   try
     if not DirectoryExists(DBLocation+'temp\') then
       CreateDir(DBLocation+'temp\');
     if DirectoryExists(DBLocation+'temp\') then
       FSession.PrivateDir:=DBLocation+'temp\';
   except
     on e:exception do //nada, leave the files where ever...
   end;
   FInitialized:=true;
 end
 else
   raise ENPInternal.Create('Error 2001: FSession and FDatabase is not
created. Initialization aborted!') ;
end;

function TNPDatabase.GetDatabaseName:string;
begin
 if not (FDatabase = nil) then
   Result:=FDatabase.DatabaseName
 else
   raise ENPInternal.Create('Error 2002: FDatabase is not created.')
end;

function TNPDatabase.GetSessionName:string;
begin
 if not (FSession = nil) then
   Result:=FSession.SessionName
 else
   raise ENPInternal.Create('Error 2003: FSession is not created.')
end;
******** code ********

I would be happy if you could point out any potential problems in the
above regarding multithreading problems...If you need more information,
please ask...

Thanks in advance!

Kind regards
 Thomas Eg Jørgensen
Wed, May 23 2007 7:51 AMPermanent Link

Roy Lambert

NLH Associates

Team Elevate Team Elevate

Thomas

I don't know if this could do it (I just assign the database name for the table/query) but this approach does not guarantee a unique name


   FDatabase.DatabaseName:='MyDatabase'+IntToStr(RandomRange(0,1000000));

Try using a GUID instead.

Since you're getting an AV it says to me that the component has already been zapped. This may be caused by a databasename clash, or something else freeing the session. Whilst the right approach is to identify and kill the problem it might be worth 1) wrapping the fsession.free in a try except block and 2) adding some diagnostic code in - test for the existence of the session at points through your code and lo its status.


Roy Lambert
Wed, May 23 2007 8:29 AMPermanent Link

=?iso-8859-1?Q?Thomas_Eg_J=F8rgensen?=
> I don't know if this could do it (I just assign the database name for
> the table/query) but this approach does not guarantee a unique name
>
> FDatabase.DatabaseName:='MyDatabase'+IntToStr(RandomRange(0,1000000));
>
> Try using a GUID instead.
>

Yeah i could try this....but still...what are the chances for two
databases ending up with the same number on the same time(come on, its
just math guys! Wink? But i will try the GUID instead...


> Since you're getting an AV it says to me that the component has
> already been zapped. This may be caused by a databasename clash, or
> something else freeing the session.
>

Yeah, my guess too....i just cant see how this can happen.....Even if
the "random"(0..1.000.000) code above fails it does not explain why im
getting so many AV's...i would expect a couple....but not 30-50 AVs a
day....


> Whilst the right approach is to identify and kill the problem it might
> be worth 1) wrapping the fsession.free in a try except block and 2)
> adding some diagnostic code in - test for the existence of the session
> at points through your code and lo its status.
>

I started to added exceptionhandling in the destroy-section, but i came
to my senses and saw that i was really just hiding a problem instead of
solving it...so i wrote the post in the newsgroupWink


Kind regards
 Thomas Eg Jørgensen
Wed, May 23 2007 9:01 AMPermanent Link

Roy Lambert

NLH Associates

Team Elevate Team Elevate

Thomas


Never forget your "random" number generator isn't.

Roy Lambert
Thu, May 24 2007 5:13 AMPermanent Link

"Frans van Daalen"

"Thomas Eg Jørgensen" <thomas@hest.notaplan.com> wrote in message
news:7C16D5F7-B2DA-4B53-97DA-9FA87252481E@news.elevatesoft.com...
> Hi,
>
> I've been using DBISAM for many years, and i thought that i was smart
> enough to develop a "fullblown multithreading database application" using
> DBISAM 3...guess notSmile
>
> My problem is that my threads occasionally fails with an Access violation
> at some "random" address. It always happens when the thread is trying to
> free the session at destruction. It only happens in <1% of the cases, but
> it happens! I have around 50 users which uses this program many hours a
> day(freeing a thread every minut or so) and i've recorded a couple of
> hundred incidents the last couple of weeks...I've tried to follow the
> steps here during the design:
> http://www.elevatesoft.com/dbisam4d5_multi_threaded_applications.htm
>
> The threads always use a database-class i've created. Every thread uses
> TDBISAMQuery-components for databasemanipulation. On Creation the threads
> query is connected to it's own database-class, e.g.:
> ******** code ********
> constructor TMyThread.Create(Settings: TNPSettings);
> begin
>  inherited create(True);
>
>  Priority:=tpLower;
>
>  //Initialize database object
>  FDatabase:=TNPDatabase.Create;
>  FDatabase.Initialize(settings.DatabasePath);
>
>  //initialize mainquery
>  FQuery:=TDBISAMQuery.Create(nil);
>  FQuery.DatabaseName:=FDatabase.DatabaseName;
>  FQuery.SessionName:=FDatabase.SessionName;
>
>  FQuery.SQL.Clear;
>  FQuery.SQL.Add('SELECT SOME STUFF');
>  FQuery.Prepare;
>
> end;
>


Move this to the execute. You want it to be created in the thread not in the
main thread which happens if you do this in de .create


> destructor TMyThread.Destroy;
> begin
>  FQuery.Close;
>  FQuery.UnPrepare;
>  FQuery.free;
>
>  FDatabase.free;
>
>  inherited destroy;
> end;
>


Can be moved to the end of the .execute

>
> procedure TNPDatabase.Initialize(DBLocation: string);
> begin
>  //Check wheter or not the nessary objects a created
>  if (FSession<>nil) and (FDatabase<>nil) then
>  begin
>    //Allways use auto session names
>    FSession.AutoSessionName:=true;


You could write your own like stated in the document

function GetNewSession: TDBISAMSession;
begin
  EnterCriticalSection(SessionNameSection);
  try
     LastSessionValue:=LastSessionValue+1;
     Result:=TDBISAMSession.Create(nil);
     with Result do
        SessionName:='AccountSession'+IntToStr(LastSessionValue);
  finally
     LeaveCriticalSection(SessionNameSection);
  end;
end;


>
>    FDatabase.DatabaseName:='MyDatabase'+IntToStr(RandomRange(0,1000000));
>

Why not just use the sessionname in de database name. The session is unique
so the databasename will be as well
like   FDataBase.DatabaseName := FDatabase.SessionName+'_db';

Thu, May 24 2007 5:13 PMPermanent Link

Tim Young [Elevate Software]

Elevate Software, Inc.

Avatar

Email timyoung@elevatesoft.com

Thomas,

<< constructor TMyThread.Create(Settings: TNPSettings);
begin
 inherited create(True);

 Priority:=tpLower;

 //Initialize database object >>

Don't create the objects in the Create constructor.  As Frans said, this
executes in the context of the main thread and isn't getting the benefit of
the multi-threading.   Create the objects at the beginning of the
TThread.Execute, and free then in a try..finally block inside the Execute
method.

<<  //Write destroy procedures
 FQuery.Free;
 FSession.Free;    <---//ERROR: Access Violation here!
 FDatabase.Free; >>

Free the database before the session.

Other than that, I don't see anything wrong with the code.

--
Tim Young
Elevate Software
www.elevatesoft.com

Mon, May 28 2007 5:06 AMPermanent Link

=?iso-8859-1?Q?Thomas_Eg_J=F8rgensen?=

"Frans van Daalen" <Account@is.invalid> skrev i en meddelelse
news:2AFB7F38-67E7-4DE7-B7BE-29EDAB20396B@news.elevatesoft.com...
>> constructor TMyThread.Create(Settings: TNPSettings);
>> begin
>> [SNIP]
>> end;
>>
>
> Move this to the execute. You want it to be created in the thread not
> in the main thread which happens if you do this in de .create
>

Ok...


>> destructor TMyThread.Destroy;
>> begin
>>  FQuery.Close;
>>  FQuery.UnPrepare;
>>  FQuery.free;
>>
>>  FDatabase.free;
>>
>>  inherited destroy;
>> end;
>>
>
>
> Can be moved to the end of the .execute
>

Ok...

>> procedure TNPDatabase.Initialize(DBLocation: string);
>> begin
>>  //Check wheter or not the nessary objects a created
>>  if (FSession<>nil) and (FDatabase<>nil) then
>>  begin
>>    //Allways use auto session names
>>    FSession.AutoSessionName:=true;
>
>
> You could write your own like stated in the document
>
> function GetNewSession: TDBISAMSession;
> begin
>   EnterCriticalSection(SessionNameSection);
>   try
>      LastSessionValue:=LastSessionValue+1;
>      Result:=TDBISAMSession.Create(nil);
>      with Result do
>         SessionName:='AccountSession'+IntToStr(LastSessionValue);
>   finally
>      LeaveCriticalSection(SessionNameSection);
>   end;
> end;
>

ok....i missed that one...will try it...

>>
>> FDatabase.DatabaseName:='MyDatabase'+IntToStr(RandomRange(0,1000000));
>>
>
> Why not just use the sessionname in de database name. The session is
> unique so the databasename will be as well
> like   FDataBase.DatabaseName := FDatabase.SessionName+'_db';
>

Ah yes...

Thanks for your reply...

Kind regards
 Thomas
Mon, May 28 2007 5:07 AMPermanent Link

=?iso-8859-1?Q?Thomas_Eg_J=F8rgensen?=

"Tim Young [Elevate Software]" <timyoung@elevatesoft.com> skrev i en
meddelelse
news:FEBC28FE-C18A-430B-83B4-C43AB5BF8D9B@news.elevatesoft.com...
> << constructor TMyThread.Create(Settings: TNPSettings);
> begin
>  inherited create(True);
>
>  Priority:=tpLower;
>
>  //Initialize database object >>
>
> Don't create the objects in the Create constructor.  As Frans said,
> this executes in the context of the main thread and isn't getting the
> benefit of the multi-threading.   Create the objects at the beginning
> of the TThread.Execute, and free then in a try..finally block inside
> the Execute method.
>

Ok, will try that...


> <<  //Write destroy procedures
>  FQuery.Free;
>  FSession.Free;    <---//ERROR: Access Violation here!
>  FDatabase.Free; >>
>
> Free the database before the session.
>

Hmm, ok...I didn't knew that...i will do that....

Thanks...

Kind regards
 Thomas


Image