At 19:28 06/01/2005 +0100, you wrote:
>Hi guys,
>
>While going through the shell extension part of TSVN to fix some wrong
>guards against concurrent access to the shell extension part I found that:
>
>1. critical sections aren't good enough to guard the cache. The reason for
>that is that the cache is global in the COM dll and therefore used by all
>processes concurrently. A mutex is needed to guard it.
This just doesn't make sense to me. In inproc com server is not shared by
multiple processes.
I'm going to put the following into the constructor of SVNFolderStatus and
see what I get on the debug stream:
char msg[100];
wsprintf(msg, "Constructing new status cache, processId: 0x%x\n",
GetCurrentProcessId());
OutputDebugStringA(msg);
>You can easily try this out yourself: create a simple dll which only shows
>a messagebox telling you the address of a dll global string. Then create a
>simple exe which does nothing more than load this dll (make sure it
>doesn't exit immediately so the dll stays loaded). Then start the exe
>several times - each time you start the exe another process is started and
>the dll is loaded into process space. But the dll tells you the same
>address of the global string, no matter which process it has been loaded from.
That's because every process has its own address space. The address you're
printing is not a 'global address', it only has meaning within the process
from which you're printing. (I am presuming that by 'global' string you
just meant static data, not something more sophisticated.)
Address 0x00000000 is not at the bottom of RAM in your PC, it's at the
bottom of your process address space.
>Since I've experienced crashs when just using critical sections I started
>to wonder and used mutexes for some time now. But to be sure I just wrote
>such a test dll and exe.
I don't think your test is showing you what you think it is.
>Another option would be to not make the cache dll global but a member of
>the COM object class. But then each overlay (normal, modified, added,
>removed, conflicted) and the column provider would each have their own
>copies of the cache...
The cache is not global across processes. It could be made so (because you
can do special stuff which makes static data in DLLs global) but we haven't
done this, and I don't think it would give any benefit, as it only holds
files from a single directory, and theres no reason to think the multiple
processes will be looking at the same place within the cache lifetime.
>2. Will suggested to make the function GetCachedItem() private and just
>expose GetFullStatus(). While this could be done, it would either mean a
>performance decrease or getting rid of the "show folderstatus" setting
>which I think should be left in TSVN so that people can speed up the
>overlays some more while only loosing the overlays for folders, not files.
I'd need to look at this in more detail. I don't think we should lose any
functionality.
>So what do you guys think? Should the GetCachedItem() function be made
>private? Or should we just leave it like it is now?
Leave it like it is and I'll try and work out what's going on. I'm going
to go and see if I can find a version of SVNFolderStatus in the repos which
uses CritSecs and see if there's anything amiss.
Cheers,
Will
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Thu Jan 6 20:00:33 2005