|
|
|
Just a quick observation. The implementation of getting the blame info in a background thread has a race condition that could lead to a crash or corruption. When the blame thread is created, there is no way for it to know if the TSvnHistoryItem has been freed. So if the blame gathering takes a really long time there is an increased chance that the TSvnHistoryItem could have been freed and then the reference to that instance in the blame thread is now stale and subsequent calls on that instance could cause bad things to happen.
One way to fix that is to remove the "FreeOnTerminate" state and keep a reference to that thread within the TSvnHistoryItem. So if the TSvnHistoryItem's destructor is called, it can set a flag that it is being destroyed so that the next call to the BlameCallback method can set Cancel to true. Then in the destructor, simply call FBlameThread.Free which will block until the thread completes.
Allen Bauer |
Homepage |
07.15.06 - 1:12 am | #
|
|
Allen, many thanks for your comment. You are, of course, right. I made the wrong assumption that TSvnHistoryItem is not freed anywhere as long as the history provider is alive. But of course it can be.
I had the gut feeling that something's not kosher with the blame thread code, but I was busy with other projects lately.
I'll try to fix it this weekend.
Thanks again for your valuable input.
TOndrej |
Homepage |
07.15.06 - 11:13 am | #
|
|
Every time (reproducibly) I try to free the thread from its OnTerminate event handler, the IDE freezes. It looks like a thread deadlock. From the CPU window, it seems it has to do with Deskutil.SetFocusHook; I don't have the source code.
However, if I use FreeOnTerminate := True; in the thread's constructor and in the OnTerminate event handler I only set the thread reference to nil, I don't get a deadlock (so far). I'll have to test it a bit more. If it seems to work I'll commit my changes to the repository at SourceForge.
TOndrej |
Homepage |
07.17.06 - 8:04 am | #
|
|
After some analysis of the existing code I've come to the conclusion that in order to provide a correct support for multithreading I have to redesign the classes a little bit. For example, I believe the "client context" structure (PSvnClientCtx) needs to be set up separately for each thread. I'd like to create a nice wrapper class for this, similar to Delphi's database session components. It's probably going to take a few days (I don't have much free time these days) but hopefully I'll be able to do it right this time.
TOndrej |
Homepage |
07.17.06 - 2:16 pm | #
|
|
Commenting by HaloScan
|