Slow Connection with IdHttp give problem in Thread
Rej
I have a particular problem when connected to a verrrry slow server.
When i try to download a file with a ThreadedForm including a idHttp with a call to GET it can take more than 30 seconds between idHTTP.OnWork calls with a slow connection. If i lose the focus on the MainForm by clicking on the ThreadedForm i cannot get back to my MainForm until the file is totally downloaded, this can take an hour...
As a solution i included a call in the OnWork section to give back the focus to and updating my MainForm, but even then it can take more than 30 seconds to get back in control.
This seems related to the blocking property of the idHTTP. That was the main reason why i used a Thread for downloading, to be able to navigate meantime with the browser of the MainForm while waiting the end of download.
Is there a way to be able to move my ThreadForm without losing control by loosing focus. There seems to be no solution on the idHTTP side, it wait for the packets for eternity with no way to process any code. I created an extra Antifreeze on the ThreadedForm just for testing, it did not seem to help at all.
From what i saw in the documentation only one should be in the application, but it is not clear about the extra threads...
I even tried to set back the focus to the MainForm in the ThreadForm.OnActivate but this was creating a problem with the ThreadForm instead...
If someone could help, i will greatly appreciate...
Rej
Remy Lebeau (TeamB)
"Rej" <Regent...@Sympatico.ca> wrote in message
news:40fd...@newsgroups.borland.com...
> When i try to download a file with a ThreadedForm
> including a idHttp with a call to GET it can take more than
> 30 seconds between idHTTP.OnWork calls with a slow
> connection. If i lose the focus on the MainForm by clicking on
> the ThreadedForm i cannot get back to my MainForm until the
> file is totally downloaded, this can take an hour...
Please show your actual code. There is something wrong with it. Such a
freeze suggests that you are executing the Get() in the context of the main
VCL thread, not the worker thread, without allowing the main thread to
continue processing new messages.
> This seems related to the blocking property of the idHTTP.
More likely a design bug in your own code. If you were truely calling Get()
in a worker thread then you would not be getting such freezes.
> I created an extra Antifreeze on the ThreadedForm just
> for testing, it did not seem to help at all.
TIdAntiFreeze only works in the main VCL thread, it is disabled when used in
worker threads.
Gambit
Martin James
>
> I hope that someone here have some experience with multithread
simultaneous download with idHTTP components.
I have serveral apps that do this -they poll multiple servers &
analyse/summarise/aggregate the results onto one form
> I have a particular problem when connected to a verrrry slow server.
I don't have much experience of this - my apps are all on private LAN/WANs.
> When i try to download a file with a ThreadedForm including a idHttp with
a call to GET it can take more than 30 seconds between idHTTP.OnWork calls
with a slow connection.
Probably :)
If i lose the focus on the MainForm by clicking on the ThreadedForm i cannot
get back to my MainForm until the file is totally downloaded, this can take
an hour...
Your app has bugs for certain, probably in inter-thread comms.
> As a solution i included a call in the OnWork section to give back the
focus to and updating my MainForm, but even then it can take more than 30
seconds to get back in control.
Not necessary, (unless you have bugs/desing problems). The word 'call'
above is a bit worrying. You cannot call another thread, only signal it.
Only the main thread should be calling VCL methods. If you have no
synchronize/sendMessage/postMessage or other inter-thread cmms, you app will
behave oddly, if not die.
> This seems related to the blocking property of the idHTTP. That was the
main reason why i used a Thread for downloading, to be able to navigate
meantime with the browser of the MainForm while waiting the end of download.
There is no problem with running TidHTTP in threads. With good inter-thread
comms, the main form and/or other forms are just not affected at all. I
posted an MDI demo of this some little while ago for someone else,
'multiHTTP' in attachments.
> Is there a way to be able to move my ThreadForm without losing control by
loosing focus.
Fix your design problems :)
There seems to be no solution on the idHTTP side, it wait for the packets
for eternity with no way to process any code. I created an extra Antifreeze
on the ThreadedForm just for testing, it did not seem to help at all.
No 'antiFreeze' needed.
Rgds,
Martin
Rej
I have to tell you that for a fast connection there is no problem at all because the GET is short in time, the problem reside only in the GET phase...
OK there is the portion of interest of my code, hoping it will help:
//===================================================
TRejThread = class(TThread)
private
Fform : TForm1;
FNewForm : TForm;
FStatus : TPanel;
FStartTime : TDateTime;
Fnumber : Integer;
FPageURL : String;
FDataList: TListBox;
FLogText : String;
FCloseThread : Boolean;
TaskProgressBar : TProgressBar;
Procedure TryToClose(Sender: TObject; var CanClose: Boolean);
Function ExtractNameFromURL(InString : String) : String;
Function FileEqualStream(File1: String; Stream1 : TMemoryStream) : Boolean;
Function GetUniqueFile(Var OriginalName : String; TestStream : TMemoryStream) : Boolean;
procedure IdHTTP2WorkBegin(Sender: TObject; AWorkMode: TWorkMode; const AWorkCountMax: Integer);
Procedure IdHTTP2Work(Sender: TObject; AWorkMode: TWorkMode; const AWorkCount: Integer);
procedure IdHTTP2WorkEnd(Sender: TObject; AWorkMode: TWorkMode);
protected
procedure Execute; override;
public
constructor Create(MyForm : TForm1; MyNumber : Integer; PageURL : String; DataList: TListBox);
end;
//===================================================
constructor TRejThread.Create(MyForm : TForm1; MyNumber : Integer; PageURL : String; DataList: TListBox);
begin
inherited Create(True);
Fform := MyForm;
Fnumber := MyNumber;
FDataList := DataList;
FPageURL := PageURL;
FCloseThread := False;
Priority := tpLowest;
FreeOnTerminate := True;
Resume;
end;
procedure TRejThread.IdHTTP2Work(Sender: TObject; AWorkMode: TWorkMode;
const AWorkCount: Integer);
Var
X, s, m, h : Cardinal;
MyReal : Real;
MyString : String;
begin
If FCloseThread Then
Begin
TIdHTTP(Sender).Disconnect;
Exit;
End;
TaskProgressBar.Position := AWorkCount;
MyReal := MilliSecondsBetween(Now, FStartTime);
MyReal := MyReal / 1000;
If MyReal > 0.5 Then
Begin
X := Trunc(AWorkCount / (MyReal * 1024));
MyReal := MyReal * (TaskProgressBar.Max - AWorkCount);
s := Trunc(MyReal / AWorkCount);
If s > 59 Then
Begin
m := s div 60;
s := s mod 60;
If m > 59 Then
Begin
h := m div 60;
m := m mod 60;
MyString := Format('%dh%2.2dm%2.2ds)',[h,m,s]);
End
Else
MyString := Format('%dm%2.2ds)',[m,s]);
End
Else
MyString := Format('%ds)',[s]);
FStatus.Caption := IntToStr(X) + 'Ko/s (Délai=' + MyString;
End;
FNewForm.Update;
end;
procedure TRejThread.IdHTTP2WorkBegin(Sender: TObject; AWorkMode: TWorkMode;
const AWorkCountMax: Integer);
begin
FStartTime := Now;
TaskProgressBar.Position := 0;
TaskProgressBar.Max := AWorkCountMax;
FNewForm.Update;
end;
procedure TRejThread.IdHTTP2WorkEnd(Sender: TObject; AWorkMode: TWorkMode);
begin
FNewForm.Update;
end;
procedure TRejThread.Execute;
Type
TImageElement = Record
ImgIndex : Integer;
Img : TBitmap;
End;
PImageElement = ^TImageElement;
Var
Label1 : TLabel;
MyList : TStringList;
i, x : Integer;
TheFilename,
TheURL,
MyString : String;
RejStream : TMemoryStream;
idHTTP2 : TidHTTP;
Begin
MyList := TStringList.Create;
MyList.Sorted := False;
If FDataList = Fform.ImageList Then
Begin
MyList.Assign(fDataList.Items); //Copy Items to be saved.
For i := 0 To MyList.Count-1 Do
MyList.Objects[i] := TObject(PImageElement(MyList.Objects[i]).ImgIndex);
End
Else If FDataList = Fform.LinkList Then
Begin
For i := 0 To fDataList.Count-1 Do
If fDataList.Selected[i] Then
MyList.Add(fDataList.Items[i]);
End
Else
Begin
MyList.Free;
Exit;
End;
FNewForm := TForm.Create(Nil);
With FNewForm Do
Begin
Width := ThreadWidth;
ClientHeight := 32;
FormStyle := fsNormal;
WindowState := wsNormal;
BorderIcons := [biSystemMenu, biMinimize];
Caption := '#' + IntToStr(FNumber);
x := Screen.Width Div Width; {MaxColCount}
If FNumber <= x Then
Begin
Top := 0;
Left := (FNumber -1) * Width;
End
Else
Begin
Top := Height;
Left := ((FNumber -x) -1) * Width;
End;
OnCloseQuery := TryToClose;
OnActivate := GettingFocus;
Fstatus := TPanel.Create(FNewForm);
Fstatus.Align := alClient;
Fstatus.Parent := FNewForm;
Fstatus.Font.Name := 'MS Serif';
Fstatus.Font.Size := 7;
Fstatus.Font.Color := clMaroon;
FStatus.Caption := 'Attente de réception...';
Fstatus.Visible := True;
Label1 := TLabel.Create(Fstatus);
Label1.Parent := Fstatus;
Label1.Height := 8;
Label1.Align := alTop;
Label1.Alignment := taCenter;
Label1.WordWrap := False;
Label1.Font.Color := clNavy;
Label1.Transparent := True;
TaskProgressBar := TProgressBar.Create(Fstatus);
TaskProgressBar.Parent := Fstatus;
TaskProgressBar.Height := 8;
TaskProgressBar.Align := alBottom;
TaskProgressBar.Visible := True;
Show;
//Fform.SetFocus;
UpdateMainForm;
End;
RejStream := TMemoryStream.Create;
idHTTP2 := TidHTTP.Create(FNewForm);
idHTTP2.ReadTimeout := 30000;
IdHTTP2.OnWorkBegin := IdHTTP2WorkBegin;
IdHTTP2.OnWork := IdHTTP2Work;
IdHTTP2.OnWorkEnd := IdHTTP2WorkEnd;
Try
For i := 0 to MyList.Count -1 Do
Begin
If FCloseThread Then
Exit;
MyString := MyList[i];
If MyString <> '' Then
Begin
If FDataList = Fform.ImageList Then
Begin
x:= Pos(Leader1, MyString);
If x = 1 Then
MyString := Copy(MyString, Length(Leader1)+1 +13 , 255)
Else
Begin
x:= Pos(Leader2, MyString);
If x = 1 Then
MyString := Copy(MyString, Length(Leader2)+1 +13 , 255);
End;
If x = 1 Then
Begin
TheUrl := 'http://' + MyString;
MyString := ExtractNameFromURL(MyString);
TheFileName := WorkingDir + ImageDir + MyString;
End
Else If FPageURL = Fform.WebBrowser1.LocationURL Then
Begin
x := Integer(MyList.Objects[i]);
Fform.theImg := Fform.htmldoc.images.Item(x,'') as IHTMLimgElement;
TheUrl := Fform.theImg.href;
If Pos(Fform.theImg.NameProp, 'images_res.gif,nav_first.gif,nav_previous.gif,nav_current.gif,nav_page.gif,nav_next.gif,nav_last.gif') > 0 Then
Continue
Else
TheFileName := WorkingDir + ImageDir + Fform.theImg.NameProp;
End;
Label1.Caption := 'Lecture de l''image ' + IntToStr(i+1) + '/' + IntToStr(MyList.Count);
End
Else If FDataList = Fform.LinkList Then
Begin
TheUrl := MyString;
MyString := ExtractNameFromURL(MyString);
TheFileName := WorkingDir + LinkDir + MyString;
Label1.Caption := 'Lecture du lien ' + IntToStr(i+1) + '/' + IntToStr(MyList.Count);
End;
Try
FNewForm.Update;
RejStream.Clear;
idHTTP2.Get(TheURL, RejStream);
If GetUniqueFile(TheFileName, RejStream) Then
Begin
RejStream.SaveToFile(TheFileName);
If FNumber < 10 Then
FLogText := 'T0' + IntToStr(FNumber)
Else
FLogText := 'T' + IntToStr(FNumber);
FLogText := FLogText +
': Sauvegarde de l''item #' + intToStr(i+1) +
' "' + TheFileName + '"';
EnterCriticalSection(Form1.LogFileSection);
WriteLn(LogFile, FLogText);
LeaveCriticalSection(Form1.LogFileSection);
End;
Except
On E:Exception Do
Begin
If FNumber < 10 Then
FLogText := 'T0' + IntToStr(FNumber)
Else
FLogText := 'T' + IntToStr(FNumber);
FLogText := FLogText + ': ERREUR avec l''item #' + intToStr(i+1) +
' "' + TheFileName + '"('+ TheUrl +')';
EnterCriticalSection(Form1.LogFileSection);
WriteLn(LogFile, FLogText);
LeaveCriticalSection(Form1.LogFileSection);
End;
End;
End;
TaskProgressBar.Position := i+1;
TaskProgressBar.Update;
End;
Finally
RejStream.Free;
idHTTP2.Free;
MyList.Free;
TaskProgressBar.Free;
Label1.Free;
FNewForm.Release;
ThreadArray[FNumber] := 0;
End;
end;
//===================================================
P.S. This is my first MultiThread program so be indulgent...
===================================================
"Remy Lebeau \(TeamB\)" <gambit47...@no.spam.yahoo.com> wrote:
>
>Please show your actual code. There is something wrong with it. Such a
>freeze suggests that you are executing the Get() in the context of the main
>VCL thread, not the worker thread, without allowing the main thread to
>continue processing new messages.
>
>> This seems related to the blocking property of the idHTTP.
>
>More likely a design bug in your own code. If you were truely calling Get()
>in a worker thread then you would not be getting such freezes.
>
>Gambit
>
Rej
My access to the main thread logfile is true a CRITICAL SECTION and all others infos are transfered to local thread variables at thread creation.
If you can find the bug in my code it would be greattly appreciated...
P.S. This is my first MultiThread program so be indulgent...
=====================================================
Martin James
Uhh.. it's a huge chunk of code.
There are a *lot* of unsafe VCL interactions in it. A lot.
As a first step, looking at the constructor and the start of the execute
method of TRejThread, you pass in a listBox and, depending on the instance
of the listBox, you copy.assign/whatever some stuff into MyList.
Then you create a form. You cannot do this here because the VCL is not
thread-safe.
As an initial suggestion - do not create the form in the thread.
Instead of creating a thread with those parameters, create the form instead.
Analyse/iterate the listBox in the form constructor or formCreate handler.
Do all that form property setting stuff in the formCreate handler, *not* in
the thread. Then create the thread from the form, passing in myList as a
parameter.
Now you have a form, created in the main thread, (ie, sage), with a thread
so service it.
Look then at the 'synchronize' method of TThread and the
sendMessage/postMessageAPIs and the TidSync/TidNotify Indy methods.
Use your choice these methods for *all* interactions with the form. from the
execute method or onWork handlers. Do not call all those VCL methods - they
may seem to work at first, but eventually, you will have a problem.
If you meed a thread bound to a form, create the form first & have the form
create the thread - it's hugely easier & safer that way round!
Rgds,
Martin
Rej
"Martin James" <mjames...@dial.pipex.com> wrote:
>
>As a first step, looking at the constructor and the start of the execute
>method of TRejThread, you pass in a listBox and, depending on the instance
>of the listBox, you copy.assign/whatever some stuff into MyList.
It is the list of all the files to download with that specific thread...
>
>Then you create a form. You cannot do this here because the VCL is not
>thread-safe.
>
>As an initial suggestion - do not create the form in the thread.
>
If i do not create the form in the thread how can i create an independant form while i'm working in the MainForm. I was beleiving that i had to put a form in a thread so it will work independantly from the MainForm. I was using 16 forms, each one using one idHTTP downloading a list of files. Maybe it's me who do not understand well the TThread processing in Win9x. I never had to work in more than one form at a time before...
>Instead of creating a thread with those parameters, create the form instead.
>Analyse/iterate the listBox in the form constructor or formCreate handler.
>Do all that form property setting stuff in the formCreate handler, *not* in
>the thread. Then create the thread from the form, passing in myList as a
>parameter.
>
>Now you have a form, created in the main thread, (ie, sage), with a thread
>so service it.
>
>Look then at the 'synchronize' method of TThread and the
>sendMessage/postMessageAPIs and the TidSync/TidNotify Indy methods.
>
After experimenting with SYNCHRONIZE in the past i have founded a CRITICAL SECTION more reliable, do you suggest that i do not use critical section?
>Use your choice these methods for *all* interactions with the form. from the
>execute method or onWork handlers. Do not call all those VCL methods - they
>may seem to work at first, but eventually, you will have a problem.
>
>If you meed a thread bound to a form, create the form first & have the form
>create the thread - it's hugely easier & safer that way round!
>
Ok i will give it a try, i will convert my code. It is already a 55,000 lines project, so i hope it will work with that approach. I will post a feedback after testing.
Thank you for your time.
Rej
Martin James
>
> Thank you for your answer, but this rise more questions to me. But first i
would like to comment:
I'll try to answer :)
>
> It is the list of all the files to download with that specific thread...
OK. There will be more than one thread? Are all the threads the same?
> >
> >Then you create a form. You cannot do this here because the VCL is not
> >thread-safe.
> >
> >As an initial suggestion - do not create the form in the thread.
> >
>
> If i do not create the form in the thread how can i create an independant
form while i'm working in the MainForm.
TnotMainForm.create(self);
I was beleiving that i had to put a form in a thread so it will work
independantly from the MainForm. I was using 16 forms, each one using one
idHTTP downloading a list of files. Maybe it's me who do not understand well
the TThread processing in Win9x. I never had to work in more than one form
at a time before..
As I read it, (and I have not read your code thoroughly), your form has a
progress bar. You need an instance of this form to appear while
downloading, show progress & then be freed, or hidden, right?
16 forms with progress bars will clutter up the screen a lot, but hey, it's
your app!
This is one way:
OK, just create a new form in the project, lets call it 'TprogressForm'. In
the options, take it out of the autoCreate list. Use the normal form design
drag &drop to plonk on all your panels, progress bar, whatever. Add the
TRejThread class, methods etc. in the form unit. Add a TidHTTP field to the
thread class and create the TidHTTP in the thread constructor, passing nil
as the owner- (the TprogressForm does not need to know anything about the
TidHTTP). Assign the onWork etc.events to the TidHTTP instance as you
already do. Add a 'myRejThread:TRejThread' field in the TprogressForm
private data. In the TprogressForm.formCreate event, create a TRejThread
instance & store it in myRejThread.
Before, you created a thread that then tried to create a form. This is very
messy & prone to disaster because the VCL is not thread-safe. Now you have
a form that creates its own thread - this is fine. All you have to do is
create 16 instances of the TprogressForm and you will get 16 forms and 16
threads.
> >
>
> After experimenting with SYNCHRONIZE in the past i have founded a CRITICAL
SECTION more reliable, do you suggest that i do not use critical section?
I wouldn't use either, but a CS is not sufficient protection for VCL
operations, even if you code it correctly - your CS seems to be protecting
only a log file writeLn & not the dozens of other VCL operations you are
performing. 'Synchronize' is better but does indeed have some concerns in
some cases - it is a hard sync point that prevents the thread from
proceeding until the synchronized method has been executed. This can be a
performance bottleneck. There are alos problems with deadlocks in
poorly-written apps that wait for thread results with 'waitFor', (something
else on my black list).
I prefer postMessage for all my work thread > main thread comms.
> >Use your choice these methods for *all* interactions with the form. from
the
> >execute method or onWork handlers. Do not call all those VCL methods -
they
> >may seem to work at first, but eventually, you will have a problem.
> >
> >If you meed a thread bound to a form, create the form first & have the
form
> >create the thread - it's hugely easier & safer that way round!
> >
>
> Ok i will give it a try, i will convert my code. It is already a 55,000
lines project, so i hope it will work with that approach. I will post a
feedback after testing.
It will, eventually.
55,000 lines and only one form!
I strongly suspect that your lack of experience with threading is making
things much harder than they need to be. For example, do you really need
to create and destroy all those forms and threads all the time? It is not
unusual to create 16 forms, and threads, like the above, at app startup &
merely hide the forms until they are given work. The threads never
terminate until app shutdown - they loop around waiting for work on either a
windows message queue or some other waitable queue, upon which their bound
form posts objects containing fields for the URL, server, response,
whatever. The threads get the requests, perform them & postMessage the
replies back to the forms.
Another easier-to-read possibility is one form with 16 progress bars and 16
thread instances, all waiting on one work request queue.
Rgds,
Martin
Rej
Thank you Martin,
I cannot tell you how much i appreciate your responses. They are clear and constructive.
I had already converted my approach with your guidelines before reading your last response, and it his working a lot better. I had some difficulties to find a way cancelling a STUCK thread that was not getting an answer fast enough in the idHTTP.GET process. After a while i decided to make a idHTTP.DISCONNECT to solve the problem, so it will end the thread operation. With slow connection it can take more than a minute between updates.
By the way my windows are so little that they do not clutter too much the screen in 1024X800, they are only 128X64 pixels.
"Martin James" <mjames...@dial.pipex.com> wrote:
>
>OK. There will be more than one thread? Are all the threads the same?
>
Yes all forms and threads are identicals.
>
>As I read it, (and I have not read your code thoroughly), your form has a
>progress bar. You need an instance of this form to appear while
>downloading, show progress & then be freed, or hidden, right?
>
>16 forms with progress bars will clutter up the screen a lot, but hey, it's
>your app!
>
>This is one way:
>
>OK, just create a new form in the project, lets call it 'TprogressForm'. In
>the options, take it out of the autoCreate list. Use the normal form design
>drag &drop to plonk on all your panels, progress bar, whatever. Add the
>TRejThread class, methods etc. in the form unit. Add a TidHTTP field to the
>thread class and create the TidHTTP in the thread constructor, passing nil
>as the owner- (the TprogressForm does not need to know anything about the
>TidHTTP). Assign the onWork etc.events to the TidHTTP instance as you
>already do. Add a 'myRejThread:TRejThread' field in the TprogressForm
>private data. In the TprogressForm.formCreate event, create a TRejThread
>instance & store it in myRejThread.
>
Exactly what i had done before reading you... Ways of programming are not so different around the world, from what i can read... (Except for my wrong thinking about threads, oops... Shame on me...)
>Before, you created a thread that then tried to create a form. This is very
>messy & prone to disaster because the VCL is not thread-safe. Now you have
>a form that creates its own thread - this is fine. All you have to do is
>create 16 instances of the TprogressForm and you will get 16 forms and 16
>threads.
>
Yes i saw that first hand, but i did not had so many problems will the older version of the Indy Objects. After ungrading them because of eMail problems i was getting a lot of new problems with different parts of my new program, merely because of there new ways of working...
>>
>> After experimenting with SYNCHRONIZE in the past i have founded a CRITICAL
>SECTION more reliable, do you suggest that i do not use critical section?
>
>I wouldn't use either, but a CS is not sufficient protection for VCL
>operations, even if you code it correctly - your CS seems to be protecting
>only a log file writeLn & not the dozens of other VCL operations you are
>performing. 'Synchronize' is better but does indeed have some concerns in
>some cases - it is a hard sync point that prevents the thread from
>proceeding until the synchronized method has been executed. This can be a
>performance bottleneck. There are alos problems with deadlocks in
>poorly-written apps that wait for thread results with 'waitFor', (something
>else on my black list).
>
>I prefer postMessage for all my work thread > main thread comms.
>
The only interface i was needing was:
1- Update the ProgressBar to see the situation of the download
2- Write in the MainLogFile your progress in the list
3- Close yourself after completing your download list
4- Let me close it if i need to
So i was using the critical section for writing in MainLogFile.
And because each form and each thread where privately bound together i was updating the rest directly... For now everything seems ok with 16 forms/threads downloading at approximately 30Kbyte/sec.
>
>55,000 lines and only one form!
>
Yes it is a very specialize application oriented on internet security. There is some extra windows for configuration and managment but for the internet and download/upload only one needed with multiple display panels.
>Another easier-to-read possibility is one form with 16 progress bars and 16
>thread instances, all waiting on one work request queue.
>
There is no more room on my form, believe me...
Thank you again for your suggestions, it had helped me a lot to this point. I have to test a little more to be sure everything is ok. If not i know where to go...
Rej
P.S. Do you have some expertise with sending a HTML email and simultaneous Attachments with a idSMTP? There is supposed to be an example MailClient doing this on the Indy website, i never founded it, neither on the web...