Overprotecting multithreaded code
Running long tasks in a background thread to keep the UI responsive is one of the main purposes of multithreading. A common code pattern for doing so would look like:
procedure TMainForm.BtnClick(Sender: TObject); begin TThread.CreateAnonymousThread( procedure begin DoSomeWork; end; end).Start; end;
However, such code often needs to show the results of that long work to the user. Working with the GUI from a background thread is not thread-safe, so such code should be executed in the context of the main thread:
procedure TMainForm.BtnClick(Sender: TObject); begin TThread.CreateAnonymousThread( procedure begin DoSomeWork; TThread.Synchronize(nil, procedure begin ShowResults; end); end; end).Start; end;
ShowResults will run in the context of the main thread, the code in
ShowResults should be as minimal as possible—read: as fast as possible. Once you
enter into the
Queue) block, you are no longer
executing code in the background thread, but on the main thread. That means if the code
in that block takes a long time to run, it will block the UI thread just the
same way it would block it if you were running that piece of code directly in
the main thread.
Sometimes, in attempts to avoid threading issues, developers overprotect code in background threads and run more code than is necessary inside synchronization blocks. Thread safety can be hard, especially if the developer has only just started sailing in multithreading waters. While overprotecting code can cause other issues, like deadlocks, generally it is better to protect a bit more code than not to protect enough.
However, I have seen cases where overprotecting literally looks like this:
procedure TMainForm.BtnClick(Sender: TObject); begin TThread.CreateAnonymousThread( procedure begin TThread.Synchronize(nil, procedure begin DoSomeWork; ShowResults; end); end; end).Start; end;
In the above example, no line of code is being executed in the background thread, and everything is running in the synchronization block. Such a coding pattern completely defeats the purpose of using threads, and running it will block the UI just the same as the following code:
procedure TMainForm.BtnClick(Sender: TObject); begin DoSomeWork; ShowResults; end;
Queue instead of
Synchronize will produce similar effects, as both
execute the code in the context of the main thread. The difference is that
Synchronize will block the thread while it runs, and the thread will finish
running after the
Synchronize block is executed, while
Queue is a non-blocking
Why would anyone write code like that?
It is hard to guess how some pieces of code come to life. In this case, I don't
think that happened because the developer was overprotective. The most logical
explanation is that the developer in question was completely unaware of what
Synchronize method actually does.
Or maybe there was a misunderstanding about how
Synchronize gives you thread
safety. If you are unaware that
Synchronize executes code in the context of
the main thread, calling all code running in a background thread inside a
Synchronize block may seem like the logical thing to do. After all, you don't want
to have threading problems.
If you take a look at the example again, and imagine that you don't know anything
Synchronize works, it will seem like all the code that is supposed to
run in the background thread will run in the background thread, including the
code written in the
Though it started as a totally unimaginable piece of code, suddenly, you can see why would someone write it like that.