tortoisehg / stable (http://tortoisehg.bitbucket.org/)

TortoiseHg repository. Main line of development in "default" branch. Releases, bugfixes, and documentation improvements in "stable" branch.

Clone this repository (size: 76.0 MB): HTTPS / SSH
$ hg clone http://bitbucket.org/tortoisehg/stable/
follow

With the last preview (TortoiseHg-0.8-preview-090630.exe) explorer crashes when a visual comparing is requested from the context menu. Windows XP SP2. It happens in every repositories.

In another machine with the same SO and the same repositories it works fine.

Here the dbgview output

[1968] [THG] CShellExt::QueryInterface: IID_IContextMenu3
[1968] [THG] CShellExt::HandleMenuMsg2
[1968] [THG] CShellExt::QueryInterface: IID_IContextMenu3
[1968] [THG] CShellExt::HandleMenuMsg2
[1968] [THG] CShellExt::QueryInterface: IID_IContextMenu3
[1968] [THG] CShellExt::HandleMenuMsg2
[1968] [THG] CShellExt::GetCommandString: idCmd = 3, uFlags = 4
[1968] [THG] CShellExt::GetCommandString: name = vdiff
[1968] [THG] Thgstatus::update: sending 'error|uninitialized handlers: added, notinrepo' to \\.\pipe\TortoiseHgRpcServer-bc0c27107423-Moreno
[1968] [THG] ***** InitStatus: error: uninitialized handlers: added, notinrepo
Status: resolved Responsible: tortoisehg Type: bug
Milestone: none Component: cmenu Version: 0.8

Attachments

Comments and changes

#1

Giampaolo Fadel / paolof

hgtk vdiff works fine.


#2

Adrian Buehlmann / abuehl

→ Changed status from new to open.

What means "the same SO"? What is SO?

BTW, the line

***** InitStatus: error: uninitialized handlers: added, notinrepo

shows that you have too many overlay handlers installed on your system and that the added overlay icon and the question mark overlay icon will not work. Try removing overlay handlers (you can use autostart from sysinternals to remove/disable them. Use the "explorer" tab).

Note that this is unrelated to the crash you report (overlay handler is mostly separate from cmenu handler).


#3

Adrian Buehlmann / abuehl

Unfortunately, I fail to reproduce a crash with cmenu visual diff. Tried on Windows XP SP3.

My debug out:

[2164] [THG] CShellExt::QueryInterface: IID_IContextMenu3
[2164] [THG] CShellExt::HandleMenuMsg2
[2164] [THG] CShellExt::GetCommandString: idCmd = 2, uFlags = 4
[2164] [THG] CShellExt::GetCommandString: name = vdiff
[2164] [THG] CShellExt::InvokeCommand
[2164] [THG] CShellExt::InvokeCommand: idCmd = 2
[2164] [THG] DoHgtk: temp file = C:\DOCUME~1\adi\LOCALS~1\Temp\THGF3.tmp
[2164] [THG] DoHgtk: temp file adding W:\xxx2\aa\asasdfa.txt
[2164] [THG] LaunchCommand: "C:\Program Files\TortoiseHg\hgtk.exe" --nofork vdiff --listfile "C:\DOCUME~1\adi\LOCALS~1\Temp\THGF3.tmp"
[2164] [THG] LaunchCommand: in W:\xxx2\aa
[2164] [THG] CShellExt::QueryInterface: UNKNOWN CLSID
[2164] [THG] CShellExt::QueryInterface: UNKNOWN CLSID

#4

Giampaolo Fadel / paolof

→ Added attachment appcompat.txt

→ Added attachment manifest.txt

Sorry you are right! I wrote SO instead of OS (Operating System). I wrong twice: the machine that crashes has SP3 while the machine that works has SP2.

You are right again: the message

***** InitStatus: error: uninitialized handlers: added, notinrepo

has no relation with the crash, it appears even in the machine that works.

I attached the file generated by Watson, if they can help you.


#5

Adrian Buehlmann / abuehl

I pushed http://bitbucket.org/abuehl/thg-stable/changeset/8870759a24f3 (will send a pull request email to Steve).

The return value of CreateFileA call was not checked.

I don't know if this is related to your crash, though.


#6

Giampaolo Fadel / paolof

I don't think this help. Your modification is inside the DoHgtk function which is called by InvokeCommand function. But the crash happens before the call to InvokeCommand, in fact in my dbgview output there isn't the message

...
[THG] CShellExt::InvokeCommand
...

It is curious because all the other commands work fine and in the ContextMenu.cpp code they are threated in the same way.

This may help you: if I promoted the vdiff command to the top level of menu the crash happens just after the click to the right button of mouse, when the menu is built.


#7

Steve Borho / sborho

→ Changed component from nothing to cmenu.

Are you using a cmenu translation? Perhaps it has problems loading the vdiff help text.


#8

Giampaolo Fadel / paolof

→ Changed status from open to resolved.

Found!

The problem is exactly related with the translation of vdiff help text. The italian version of help text is larger than the original text. If I cut the italian text all works fine.

Adrian has fixed with r3072:6be9d400438d and now with the first stable 0.8 the problem is resolved.

Just a curiosity: I have never seen working the help text of cmenu. Is it a bug? It should appear as a tooltip? Does it depends on some Windows settings?


#9

Adrian Buehlmann / abuehl

Thanks for the info. I'm glad to read it no longer crashes.

The help text is shown in status bar in explorer (while you hover around in the context menu).

You need to enable the status bar to see it. In English Windows XP this is in menu "View" (checkmark "Status Bar").

If I understand this correctly, then I fear there is still a bug? Sounds like the Italian help text is still
not treated correctly?

Maybe 6be9d400438d "fixed" this bug here just in so far that the shell extension no longer crashes explorer. Probably the root bug is still present.

I will have another look at the code.


#10

Giampaolo Fadel / paolof

Thanks, it is not a bug, it is my mistake. I always use Total Commander and I forgot the existance of explorer itself.

They works, thanks.


#11

Adrian Buehlmann / abuehl

→ Changed status from resolved to open.

There is still some bug. I installed the Italian cmenu texts and I see for example:

[1308] [THG] CShellExt::GetCommandString: idCmd = 0, uFlags = 4
[1308] [THG] CShellExt::GetCommandString: name = commit
[1308] [THG] CShellExt::GetCommandString: idCmd = 1, uFlags = 4
[1308] [THG] CShellExt::GetCommandString: name = vdiff
[1308] [THG] CShellExt::GetCommandString: error: source string length (68) exceeds target buffer size (64)

There must be some serious bug left if there is such a ridiculously small target buffer.

But at least it doesn't crash anymore for the moment.

I will investigate this.


#12

Adrian Buehlmann / abuehl

→ Changed status from open to resolved.

Since GetCommandString is a function from a COM interface, we can't do much but implement how the caller calls it. It seems explorer is calling us with a length of the target buffer that is shorter than our string. Best we can do is truncate it.

Interestingly, I don't see any truncated strings in the ui.

Implemented in r3098:82b700937130 ("shellext: copy truncated strings in GetCommandString").


#13

Giampaolo Fadel / paolof

I think, in order to avoid any problem, it's better to reduce the length of translated string to 64 chars (is this the limit?) directly on the file .reg. Remember, at the beginning of this issue, that in some machine explorer crashed while in others no. So the behavior of explorer is a little bit unpredictable.

What do you think?


#14

Adrian Buehlmann / abuehl

Yes, Microsoft recommends a limit of 40 characters for the help texts [1], as I have noted in r3110:8fccf0215997 , which will be included in the next release (0.8.1).

There is nothing unpredictable here, as the original crash reason (which affected only a prerelease of 0.8) was because we didn't respect the size of the target buffer and the Italian help text for vdiff was exeeding that length. This had already been fixed for the 0.8 release ( r3072:6be9d400438d ).

[1] http://msdn.microsoft.com/en-us/library/bb776094%28VS.85%29.aspx: "The help text is a description of the command that Microsoft Windows Explorer displays in its status bar. It should be reasonably short (under 40 characters)."


#15

Giampaolo Fadel / paolof

40 characters ? May be hard to write down an help string in only 40 characters expecially in italian language, but I'll try.

BTW, what happens if the translated text is longer than 40 characters? Looking to r3110:8fccf0215997 you pass the string the same and then trace out a warning message. Is it right?

Pay attention, even in *en-US.reg file some strings exceed 40 chars. And even the hard coded strings in ContextMenu.cpp are so.


#16

Adrian Buehlmann / abuehl

Yes, it is right. The only hard limit is the size of the target buffer, which is not specified. The requirement spec of the function (see the msdn link I provided) does not require to truncate strings at 40 characters (or any other size).

Yes, there is already one help string in the English hard coded texts that is longer than 40 characters. I've noticed that too.

I have added the warning in the code so that it is easier to find these. Relevant debug out excerpt:

[3136] [THG] CShellExt::GetCommandString: name = "recover"
[3136] [THG] CShellExt::GetCommandString: warning: length of help text is 41, which is not reasonably short (<40)
[3136] [THG] CShellExt::GetCommandString: res = 0, pszName = "General repair and recovery of repository"

The original implementation of the cmenu code did not obey any limits. Since I started hacking on the cmenu part, I tried to improve it. We are gradually maturing the code, as it evolved from experimental status as published by TK Soh (which provided the excellent starting point for the new C++ shell extension) to its first release (0.8). Nevertheless, the quality of this first release is not that bad, IMHO.

40 characters is indeed very short. I understand that it is difficult to stay under 40 in verbose languages like Italian. German (my domain, I'm Swiss-German speaking -- Bernese German, to be axact) has the same problem :)

I don't know why Microsoft is asking to stay under 40 characters. Technically, the limit seems to be higher (68 for the target buffer size, as we have seen in one case -- but this is no official limit).


#17

Giampaolo Fadel / paolof

I fully agree with you. Let's cut the translated string in order to improve a bit the quality of the product.


#18

Adrian Buehlmann / abuehl

I've just pushed http://bitbucket.org/abuehl/thg-stable/changeset/cef8817c9f08/ which prints the target buffer sizes in debug output.

Some examples below:

[2800] [THG] CShellExt::GetCommandString: idCmd = 0, uFlags = 4 (GCS_VERBW), cchMax = 64
[2800] [THG] CShellExt::GetCommandString: name = "commit"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "commit"
[2800] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 4 (GCS_VERBW), cchMax = 64
[2800] [THG] CShellExt::GetCommandString: idCmd not found
[2800] [THG] CShellExt::GetCommandString: res = 1, pszName = ""
[2800] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 4 (GCS_VERBW), cchMax = 64
[2800] [THG] CShellExt::GetCommandString: idCmd not found
[2800] [THG] CShellExt::GetCommandString: res = 1, pszName = ""
[2800] [THG] CShellExt::GetCommandString: idCmd = 0, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "commit"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "Commit changes in repository"
[2800] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: idCmd not found
[2800] [THG] CShellExt::GetCommandString: res = 1, pszName = ""
[2800] [THG] CShellExt::GetCommandString: idCmd = 14, uFlags = 1 (GCS_HELPTEXTA), cchMax = 260
[2800] [THG] CShellExt::GetCommandString: idCmd not found
[2800] [THG] CShellExt::GetCommandString: res = 1, pszName = ""
[2800] [THG] CShellExt::GetCommandString: idCmd = 1, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "status"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "Repository status & changes"
[2800] [THG] CShellExt::GetCommandString: idCmd = 2, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "shelve"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "Shelve or unshelve file changes"
[2800] [THG] CShellExt::GetCommandString: idCmd = 3, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "vdiff"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "View changes using GUI diff tool"
[2800] [THG] CShellExt::GetCommandString: idCmd = 4, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "log"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "View change history in repository"
[2800] [THG] CShellExt::GetCommandString: idCmd = 4, uFlags = 5 (GCS_HELPTEXTW), cchMax = 600
[2800] [THG] CShellExt::GetCommandString: name = "log"
[2800] [THG] CShellExt::GetCommandString: res = 0, pszName = "View change history in repository"
[2800] [THG] CShellExt::GetCommandString: idCmd = 4, uFlags = 4 (GCS_VERBW), cchMax = 64
[2800] [THG] CShellExt::GetCommandString: name = "log"

As you can see in the case for the help texts (GCS_HEPTEXTW), the target buffer seems to be always reasonably large (260, 600).

The original (experimental) implementation GetCommandString didn't care for the uFlags parameter and always returned the help text (even if the request was a GCS_VERBW, which seems to provide a shorter target buffer -- 64 in the examples above).


#19

Giampaolo Fadel / paolof

Interesting. May the suggested limit of 40 characters is related to the GUI layout more than a buffer size. I don't know. In any case I'll cut the text to 40 chars. If in the future we discover something I'll revert the texts.


#20

Adrian Buehlmann / abuehl

Since we started to obey cchMax, we have no problem with buffer sizes (which was done for 0.8 in r3072:6be9d400438d , as I said).

The 40 char limit is an unrelated concept indeed.


#21

Giampaolo Fadel / paolof

You are right the 40 chars limits seems an advice concerning readability, the real limit is cchMax.

BTW, I cuted the strings to 40 chars and this new version is better than the previous one. So I sent a patch to Steve.


Add comment / attachment

Show/hide preview

Verification: Please write the text from the image in the box (letters only)

captcha

Is that you, Humanoid? Is this me?