Hi all,
in my hunt for crashes, I found another one, this time a bit more
complicated one.
As far as I know, TSVN is running as STA COM component (ref:
http://svn.haxx.se/tsvn/archive-2007-04/0141.shtml). That means, that
only the thread that created the component can use it. However, if two
threads create two instances of the component, they can use it
simultaneously, right?
And here comes the problem. In file IconOverlay.cpp in TortoiseShell,
there is a COM method CShellExt::IsMemberOf. This method is using some
global variables, e.g. g_filepath, and does not use any synchronization.
That means, if two instances of this component are created from two
threads and this method is called in both, we can easily get data in one
overwritten by the other. (ref:
http://msdn2.microsoft.com/en-us/library/ms809971.aspx, especially this:
*Note *Because multiple instances of an object may be created in
different apartments, global data is not thread-safe in an STA and
should be protected. Critical sections, which allow exclusive access to
a segment of code to a single thread, are most often used for this
purpose. A discussion of the different thread synchronization mechanisms
is outside the scope of this article.)
We were able to reproduce this as a crash in the latest nightly build of
TSVN (1.4.99.9900) in combination with our filemanager Altap Salamander,
which is using multiple threads to read icon overlays for it's panels.
Note, that we were able to reproduce this so far only on multicore CPUs.
Theoretically, it may be possible to reprodue this with mutliple windows
of Windows Explorer as well, although we haven't tried it yet.
Possible solutions? I see threee:
- Add synchronization to this and other methods using global variables.
The simplest, but least effective and clean.
- Change the global variables to be member variables of the COM object,
thus making them per-instance. Clean, but may impact effect of some of
the variables which work as a cache (there may be more instances in one
thread, however one thread is likely to be inspecting one directory,
multiple threads are likely to be in different directories).
- Change the global variables to be stored in TLS (thread local
storage). Clean, may be better for the effectivity of the caching variables.
Thoughts?
Regards
Tomas
Received on Thu Jun 28 10:33:20 2007