I've got a few crash reports sent in for TortoiseSVN, but the cause seems to be in DOpus:
On a context menu request, in IShellExtInit::Initialize() TSVN asks for the CFSTR_SHELLIDLIST clipboard format from the IDataObject member (second param in IShellExtInit::Initialize). That should get a valid CIDA object according to this: msdn.microsoft.com/en-us/library ... HELLIDLIST
But it seems that object is somehow corrupted. Because later when using that CIDA object and enumerating over the PIDLs one of those points to invalid memory so that a call to IShellFolder::GetDisplayNameOf() segfaults.
Since I don't get such crash reports sent in for explorer I have to assume the problem is in DOpus.
so you can analyze the problem further. I don't have the debug symbols for DOpus but I hope you keep those.
In the zip file you also find a cmd file that you can use to start WinDbg with all the required settings. If you want to use another debugger, use the url: crash-server.com:8080/public ... ED7/symsrv
to fetch the TSVN debug symbols from. The dump file also contains the info from where to get the corresponding TSVN source files (if you have svn.exe in your PATH). So you should have all the info you need.
From a very quick look it appears that the crash is on a thread created by Windows for the standard Properties dialog, rather than in the right-click context menu itself (although the Properties dialog is usually invoked via a command in the context menu, of course).
Am I looking at the right thing or going down the wrong track?
For what it's worth, Opus uses custom code for (most of) its context menus, but just calls on the usual Windows shell API to invoke Properties dialogs (which explains why there's no Opus code on the stack for this particular thread, since the Properties dialogs spawn their own threads). Of course, if we're invoking the properties dialog in the wrong way somehow then the crash could still be our fault, and the list of PIDLs given to the properties dialog will be coming from Opus indirectly, at least.
Sorry, yes you're right: it's in the property sheet handler. I got confused because it's the same code in TSVN since the IShellExtInit::Initialize() is used by almost all extension types.
But: the data passed to IShellExtInit::Initialize() is the same, and there's still invalid PIDLs passed to it.
When you call the shell APIs to show the property pages, do you pass paths or PIDLs to it? If it's PIDLs instead of paths, then the problem might lie there.
You said that a new thread is created to show the property pages. Maybe the memory of the PIDLs is released while the thread showing the property dialog is still active?
Or maybe there's something wrong with my code and I can't see the bug there. Can't be sure.
One thing I know is that all crash reports failing in the call to IShellFolder::GetDisplayNameOf() I get are from DOpus.exe. Haven't found one originating from explorer.exe (yet?).
What I noticed in the crash dump is that the PIDL count is quite high (122 in this dump). Maybe you don't reserve enough memory?
Or: explorer.exe is limited to MAX_PATH which includes PIDLs and their length. Can DOpus handle longer paths? In that case I would have to add some checks in TSVN before calling any shell API to avoid passing too long paths/PIDLs to them.
Opus will handle paths over 260 characters in a lot of places (not absolutely everywhere), so it could well be sending shell extensions PIDLs for paths over 260 characters.
I tried making a really long path:
X:\svnsrc\test area\Miscellaneous\This is a very long path indeed a very long path\It is very very long this path\Wow Such Path Very Length So Characters\Why do people have such long paths I am already bored just from typing this one\It is ridiculous really\The 260 character limit seems bad on paper but maybe path lenths this long are what's really wrong\Anyway, here's a file.txt
Then adding it to SVN, from the "This is a very long path..." part near the top (the folders above that level were already in SVN):
Where the path is truncated in the status window seems slightly different to where it seems to break down in terms of which directories are added. I've used Opus's Flat View to show the different branches at once, but the same results appear in Explorer as well:
(It's maybe of interest that the 4th dir there has no SVN Status column (not "Added" like the ones above it) and has a green tick icon indicating it's in SVN, when it's either added or not in SVN at all (I've not committed any changes here). Don't know if that's important though.
The SVN property sheet works OK, in both Opus and Explorer, up to the "It is ridiculous really" folder in my example. That's got a path length of 258 characters, so it's just under the limit. After that folder, the SVN property sheet isn't included in the Properties dialog at all (makes sense as the folders with really long paths didn't get added), but I don't see any crashes either (at least so far).
Maybe the crashes only starts if the property sheet is used on something with a longer path that is also in SVN, but I'm not sure how to add that. (Maybe with the command line tools? I've not had a chance to try yet as we've been very busy with the new Opus 11 beta today.)
It may be interesting/relevant that the Properties dialog (whether launched via Opus or Explorer) switches to using short 8.3 paths as soon as you go into the items with paths over 260 characters:
Anyway, the number of crash reports for this particular crash isn't that high: for the latest TSVN version (1.8.4) there are 23 reports so far from 6 different IDs. Which means it doesn't happen too often.
Maybe I get a dump where I can get more information out of and find the problem, but as it is now I only can see that the PIDL points to invalid memory. And since that PIDL is passed via the data object passed to the Initialize() function, I can't see any way that TSVN could corrupt that memory itself.
I had the idea of checking out a repository into a normal folder, then moving it into a really deep/long path, since that would end up with files under SVN control but with very long paths.
I tried a few things but haven't seen any crashes so far. The TortoiseSVN property sheet gets dropped from the Properties dialogs once the path gets too long, but that's expected if TSVN isn't aiming to handle such long paths, and so far I haven't been able to crash anything. So I agree, the long paths might not be the cause. (Unless it's something really subtle that takes 'luck' to reproduce, like part of the path has to be exactly 260 or 259 characters long or similar.)
It could be an interaction with a third component, maybe one that really is corrupting the memory, but I can only guess, of course.
In terms of practical things we can do that might help track it down, the only thing I can think of is that we could add an option to Opus which logged the PIDL values it was using for the Properties dialog, so the people affected could turn on that logging and then the next time they get a crash they could recover the log and we could inspect the PIDLs to see if they were valid or revealed any clues.
I guess that is only useful if you're in contact with the people seeing the crash. I don't know if the crash reports are anonymous or not. But if it would be useful for us to add that, let us know. It should be fairly simple to do.
The crash reports are anonymous. So there's no way to contact the users who sent them or find out who they are.
But users who experience a crash are redirected to a web page with their sent report, and I can add another redirect to an info page. I've already done this for this crash, users get redirected to the issue I've created in our issue tracker:
