Hi everybody,

I'm mailing you as I have a very strange behaviour of a program I'm making using FPC 2.6.2 and Lazarus from trunk (updated this morning). As far as I know it's either a compiler bug or a Synapse bug. Let me explain...

The project is a GUI for the Spack package manager used by a Linux distribution I'm co-developing (4 users actually and only for developers and only in French, so no chance you have it). The problem happens with the download manager I've created based on Synapse. The first file is downloaded correctly but when switching to second one I've got a runtime error 216 (access violation). The strange thing is when debugging with F7/F8 in the IDE, everything runs OK: no error any more (so my code should not be in cause).

You can find full source code and testing files here : svn://svn.tuxfamily.org/svnroot/geofperspage/gppsvn/trunk/0linux-gui

After checking out, you'll have to change few things in the code :
- in common/ucommon.pas: the paths given for testing would be change accordingly to your where you checked out the project
- in spackgui/frmabout.pas: I'm using the Lazarus' source code to have version info: remove it or change it accordingly. (Note : I'm looking for a way to get this without looking to Lazarus source code)

As this is very specific to Linux, this project is Linux only, sorry...

For non French speaking, most of the interface is in French, nevertheless all debugging messages, identifiers and comments are in English so this should not be a big problem.

Thank you for any help !

Geoffray Levasseur-Brandin wrote

I'm mailing you as I have a very strange behaviour of a program I'm

making

using FPC 2.6.2 and Lazarus from trunk (updated this morning). As

far as I

know it's either a compiler bug or a Synapse bug.

That is always a dangerous assumption;)

Thank you for any help !

There is quite more needed to get the code running. I can't get it compiled on my kubuntu 12.04. It complains about duplicate resources.

Anyways, looking at the code, you create a thread TDownloadListLoop that loops over the files to download. The download is done in another thread TDownloadDaemon created in TDownloadListLoop.Execute with PDownloader^.StartDownload. TDownloadListLoop.Execute then loops until PDownloader^.State = drDone. Now, in TDownloader.HTTPComplete, called from TDownloadDaemon.Execute with a synchronize you do FState := drDone followed by FDownloadFeedback(FResponse) and a FHttp.Clear. Chances are that TDownloadListLoop.Execute continues and starts a download of the next file before TDownloader.HTTPComplete does the FHttp.Clear. PDownloader^.State = drDone is too early.

Buy the way, you didn't mention if you encountered the problem with http or ftp. So above is just speculation. One of the places where it can go wrong.
In any case I don't understand your architecture. Why using a thread for the loop, do a HEAD in the same thread and then create another thread to do the GET? If you only do one download at a time why the 2nd thread TDownloadDaemon? On top of that your code becomes overly complex because methods in TDownloader are called in the context of the TDownloadListLoop thread and of the main thread (synchronize from TDownloadDaemon). A good recipe for race conditions and unreproducible errors.

Ludo

Ludo Brands wrote

There is quite more needed to get the code running. I can't get it compiled on my kubuntu 12.04. It complains about duplicate resources.

Hum maybe you have Synapse installed in your Lazarus as I'm briging it with the application (and it's appearing in project manage).

You're right this could have been a problem if download parameters are different, thank you. Unfortunately, even if this is now corrected, the original problem is not solved. Anyway the changes are commited.

Buy the way, you didn't mention if you encountered the problem with http or ftp. So above is just speculation. One of the places where it can go wrong.

The two first files are http, then two ftp files, all on the same server (look at testconf/depot.conf for the addresses). Each address will need the download of two files, paquets.db and applications.db.

This is somewhat complex, I know it, but this have been created keeping in mind that I would like to allow parallelization of downloads in the future, as you was supposing it. But I'm agree and if someone have an idea on how simplifying it and almost in a more secure way, allowing parallelization in future I would very appreciate it.

Ludo

I've made some more test: the bug appaer only if either the application or Lazarus (if started from Lazarus) is focussed...

If anybody have that king of issue, I'll try to give more platform independent code if I have the exact error.

Thanks,

Geoffray Levasseur-Brandin wrote

Hum maybe you have Synapse installed in your Lazarus as I'm briging it with the application (and it's appearing in project manage).

Synapse is not in my default path. So that is not the problem. It complained about a duplicate resource in one of the qt dirs. Haven't investigated any further.

You're right this could have been a problem if download parameters are different, thank you. Unfortunately, even if this is now corrected, the original problem is not solved. Anyway the changes are commited.

Well it can always cause a problem because FHttp.Clear clears the Header and Document. If that happens while you are executing the HEAD, strange results can happen.

Instead of re-using TDownloader.FHttp: THTTPSend for all threads, create a THTTPSend per TDownloadDaemon. You have to use multiple THTTPSend instances in any case if you want to do parallel downloads. Actually, get rid of TDownloader and do everything in TDownloadDaemon. Even the HEAD.

I've made some more test: the bug appaer only if either the application or Lazarus (if started from Lazarus) is focussed...

That is probably linked to the GUI code that takes more time painting when focused. So again indication that you still have some race conditions.

Ludo

Also, get rid of all these synchronizes. When using multiple threads, you will be blocking your downloads too much. Every sock status change, you create a string for the GUI, synchronize to update the screen, call Application.Processmessages to make sure the GUI gets updated. That is a huge waste of resources and slows down downloads considerably. On top of that you have timers that also update the GUI.
Turn this around. Have TDownloadDaemon do the complete download and nothing else. Put status in properties of TDownloadDaemon, using a critical section if you want to pass strings, and poll your download threads in the main thread to update the GUI. You have already a timer. Make it faster (fe. 200ms) and update your GUI from there. Nobody will see the difference. No more synchronize and Application.Processmessages anymore and your program flow becomes so much easier. Multi-threading is difficult and simplifying your program flow is key to getting it right.

I changed the project to GTK2 and got it running. It crashes not always at the same time but it is systematically in the synchronize in TDownloadDaemon.SockStatus and always when doing the HEAD. So that is a synchronize called inside a synchronize from the TDownloadListLoop. You should not call synchronize from the main loop. Worse, TDownloadDaemon.SockStatus calls Synchronize(FOnProgress) which calls TThread.Synchronize(self,AMethod). Self is TDownloadDaemon but since you call TDownloadDaemon.SockStatus here from the main thread, synchronize uses the wrong AThread. The first download it happens to work because there is no thread running when you do HEAD. The second time you aren't that lucky. In any case, this is an overly complex program flow and is simply asking for problems.

Ludo

Thank you for all the advices. I just removed TDownloadDeamon thread to merge it with TDownloader. And it works ! I still have to work on status/progress system which is a bit messy. If you are interested, I've just commited the changes in the svn repo.

Thank you again,

I noticed you are now calling Application.ProcessMessages in TDownloader.SockStatus. You shouldn't make that call from a thread. It will try to handle the messages from the thread. Perhaps the widget set has some protection for that and will ignore calls coming from anything but the main thread but I doubt it. It'll cause sooner or later big problems.

Get your GUI stuff out of the download thread. It pays to have clean and understandable threads.

Ludo

Ludo Brands wrote

Well it can always cause a problem because FHttp.Clear clears the Header and Document. If that happens while you are executing the HEAD, strange results can happen.

In fact a a very recent project my problem was that I misread the Synapse Docu and assumed that with a new HTTPMethod() call the send buffer ("document") would be clear. But on the contrary the send buffer is filled with the latest received document. This in fact leads to programs that mostly work but are due to very peculiar problems, as the target HTTPserver might overrun and timeout ow whatever when bombed with huge request frames.

I don't think this is very hand, but I asked the software creators and it's a feature not a bug.

Thus before using HTTPMethod() of an instance of THTTPSend a second time, you to always need to clear the document buffer before.

-Michael

Ludo Brands wrote

you create a string for the GUI, synchronize to update the screen, call Application.Processmessages to make sure the GUI gets updated.

I would suggest to do those GUI updates by QueueAsyncCall() or TThread.Queue() (TThread.Queue uses the same syntax as Synchronize and thus is rather easy to use in your case (beware of string references, though), but it might be present only in the svn version of fpc as it is rather new (because it has been existing but not documented in Delphi until rather recently. )

TThread.Queue works like TThread.Synchronize, but does not stall the thread until the mainthread has done the requested action. Thus, if no result is to be reported back or mutual access problems are lurking, it is is a lot better than Synchronize: The thread keeps running and no performance-eating ProcessMessages is necessary.

QueueAsyncCall does the same, is more versatile but uses different Syntax and is not available in Delphi.

( PostMessage / procedure ... message does the same, is available and documented in Delphi, but uses very ugly fake-Windows syntax, even if used in Linux. )

-Michael

Ludo Brands wrote

Get your GUI stuff out of the download thread....

Easy to do using TThread.Queue()....

-Michael