Can't "send-to" (MAPI) using Seamonkey and Thunderbird X86_64-bit

RESOLVED FIXED in Thunderbird 66.0

Status

--
major
RESOLVED FIXED
12 years ago
a month ago

People

(Reporter: baffoni, Assigned: mikekaganski)

Tracking

(Blocks: 2 bugs, {64bit})

Trunk
Thunderbird 66.0
x86_64
Windows
64bit
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird_esr6065+ fixed, thunderbird65 fixed, thunderbird66 fixed, seamonkey2.49esr affected)

Details

Attachments

(3 attachments, 11 obsolete attachments)

3.08 KB, text/plain
Details
15.52 KB, patch
jorgk
: review+
Details | Diff | Splinter Review
15.49 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
I installed Windows XP 64-bit, and I've lost the ability to use MAPI hand-off to Seamonkey (SeaMonkey 1.1.4 Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4) and Thunderbird version 2.0.0.6 (20070728).

Steps to recreate:
1)Install Thunderbird or Seamonkey on a Windows XP 64bit machine:
2) Set either application as the default mail client (e.g. within the client, or control panels/Internet Options/programs/default mail client).
3) right-click on a file, select send-to->mail recipient
or
Open an office application, open a new file, attempt to find "Send-to->mail recipient".

Actual results: In the first case, there is no action - no email client comes up, nor does a compose window; in the second case, there is no option to do the send-to because it can't find a MAPI client.

Expected results: a new compose window should come up with the file attached.
Component: General → MailNews: Simple MAPI
Product: Thunderbird → Core
QA Contact: general → simple-mapi
Version: 2.0 → 1.8 Branch

Comment 2

11 years ago
Any chance anyone could have a quick look at this one? I've a client who is seeing this exact behaviour with Windows XP 64-bit and Thunderbird 2.0.0.16 (32-bit). I'll try an unoffical 64-bit Thunderbird in the meantime, and see if that fixes the problem.
Product: Core → MailNews Core
(Reporter)

Comment 3

10 years ago
BTW, I'm still having this problem with thunderbird 3b2Pre.
per sid0 "the problem is probably that the xp64 explorer can't load our 32-bit proxy. the Vista explorer handles it by doing it out of proc (IIRC)"

sid0, who was the other person recently working integration issues - Dossy?
Hardware: x86 → x86_64
cc some XP users

does this still exist in current thunderbird versions?
and, what is/should be the precise relationship of bug 390331?
(Reporter)

Comment 6

7 years ago
I don't use seamonkey any more.  We started putting this into the registry to fix other MAPI issues, it may have fixed this as well:

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows Messaging Subsystem]
"MAPI"="1"
"CMC"="1"
"CMCDLLNAME"="Mapi.dll"
"CMCDLLNAME32"="Mapi32.dll"
"MAPIX"="1"
"MAPI"="1"
"MAPIXVER"="1.0.0.1"
"OLEMessaging"="1"

If no one else is seeing this problem, this is a WFM issue.
I can confirm this issue on Windows XP 32-bit version.

Updated

6 years ago
Duplicate of this bug: 809504

Comment 9

6 years ago
Confirming Windows 7 X64  So updating os and branch as it is extant on trunk.  TB21a

Note Michael Baffanis suggested registry mods made the crash take longer, but that is all.

Crash reporter does not appear. Confirming Weasley Hardmans observations in bug 809504
OS: Windows XP → All
Version: 1.8 Branch → Trunk

Updated

6 years ago
Summary: Can't "send-to" (MAPI) using Seamonkey 1.1.4 and Thunderbird 2.0.0.6 → Can't "send-to" (MAPI) using Seamonkey and Thunderbird X86_64

Updated

6 years ago
Blocks: 471090

Updated

6 years ago
Blocks: 482756

Updated

4 years ago
See Also: → bug 1175598

Updated

3 years ago
Blocks: 634233
No longer blocks: 471090

Updated

3 years ago
Duplicate of this bug: 835087

Updated

3 years ago
See Also: bug 1175598

Updated

3 years ago
Duplicate of this bug: 1175598

Comment 12

2 years ago
We learned in bug 1308813 comment #0 that
  HKLM\SOFTWARE\Clients\Mail\Mozilla Thunderbird\DLLPath
needs to be set to
  C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\mailnews\mapi\mapiDLL\mozMapi32.dll
to debug this.

So I did that. "Send to > Mail recipient" worked immediately. However, not the home-compiled version was started, but the my Daily which I have installed in C:\Program Files\Mozilla Thunderbird 52.

I changed the registry key back to C:\Program Files\Mozilla Thunderbird 52\mozMapi32_InUse.dll and lo and behold, the "send to" still works.

Next, I set the registry value to
C:\Program Files (x86)\Mozilla Thunderbird 50\mozMapi32_InUse.dll
That DLL is from the x86 TB 50 beta I have installed. "Send to" still works and TB 50 beta was started.

So back to:
C:\Program Files\Mozilla Thunderbird 52\mozMapi32_InUse.dll
and it still works starting x64 TB 52 Daily.

In short: The crash I had initially has gone away.

My wild guess would be: I have x86 and x64 versions on the machine. Fiddling with the registry value seemed to wake up Windows and now I simply can't make it fail any more.

Richard, can you check this:
Set HKLM\SOFTWARE\Clients\Mail\Mozilla Thunderbird\DLLPath to whatever it was but without the "_InUse", so in my case:
C:\Program Files\Mozilla Thunderbird 52\mozMapi32.dll
That DLL also exists. Try the "Send to". Then change it back, try again.
Or append an "x", so:
C:\Program Files\Mozilla Thunderbird 52\mozMapi32_InUse.dllx.
Try "Send to", you get an error, then remove the "x" again.

I think there is a very subtle problem. I'd be interested to know what happens if you install an x64 TB on a fresh install of Windows.
Flags: needinfo?(richard.marti)

Comment 13

2 years ago
Rainer, you can try this too.
Flags: needinfo?(RainerBielefeldNG)
Still crashes here. I also tried to use the path to my self-built 64 bit TB and started this build, and it crashed. Restoring the path let also crash my 64 bit Daily. Here your proposal didn't help.
Flags: needinfo?(richard.marti)

Comment 15

2 years ago
I've come to the conclusion that this is a Windows problem. I started my Windows 10 laptop. "Send to" did nothing. I checked the default programs, Daily was the default program. I chose it again. Suddenly "Send to" started working. It starts the x64 version installed on the laptop. No crash.

This bug here shouldn't stop us shipping TB as 64bit version. If we get more reports, we may be able to dig deeper into it. I can't reproduce the crash, so I don't know what else to try. Richard, you just have to reinstall Windows ;-)

Comment 16

2 years ago
(In reply to Jorg K (GMT+2) from comment #13)
This Bug had a strange metamorphosis from "does not work" to "Crashes" 
Keyword is 64bit, Comment 7 confirms something with 32-bit ...

I already did all tests I can do easily in "Bug 1175598 - 64Bit: Impossible to make Mail/News default application for "send from WIN Explorer", "Send document (as PDF)" ...": g1) 64Bit build problem  --   h2) Send to email recipient from WIN Files Explorer: will crash Thunderbird.
I will try to reproduce your registry results, later.
Flags: needinfo?(RainerBielefeldNG)

Comment 17

2 years ago
It happened to me again. I did:
  Right-click -> Send to > Mail recipient
and it crashed. I changed
  HKLM\SOFTWARE\Clients\Mail\Mozilla Thunderbird\DLLPath
to
  C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\mailnews\mapi\mapiDLL\mozMapi32.dll
to debug this and it became good, however, not my debug build started but instead my 64bit Daily.

I'll have to look into how to get the local debug build started.

Comment 18

2 years ago
OK, for the record, this registry file will do it:

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}\LocalServer32]
""="\"C:\\mozilla-source\\comm-central\\obj-x86_64-pc-mingw32\\dist\\bin\\thunderbird.exe\" /MAPIStartup"
(or course you need to change the path to your build)

Information can be found here:
https://dxr.mozilla.org/comm-central/rev/b020cffa188fac078bff74c9e19c98b044ea4173/mail/installer/windows/nsis/shared.nsh#307

Comment 19

2 years ago
With these two registry keys set:

[HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla Thunderbird]
@="Mozilla Thunderbird"
"DLLPath"="C:\\mozilla-source\\comm-central\\obj-x86_64-pc-mingw32\\mailnews\\mapi\\mapiDLL\\mozMapi32.dll"

[HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}\LocalServer32]
""="\"C:\\mozilla-source\\comm-central\\obj-x86_64-pc-mingw32\\dist\\bin\\thunderbird.exe\" /MAPIStartup"

I get |Sent to > Mail recipient| to start my debug build and at times it crashes. Attached a stack.

So it crashes in nsAppShell::ProcessNextNativeEvent() inside ::DispatchMessageW(&msg):
https://dxr.mozilla.org/mozilla-central/rev/d7b6af32811bddcec10a47d24bd455a1ec1836fc/widget/windows/nsAppShell.cpp#375

I have no idea how to debug this further, so let's see who can help.

I'll try Jim and Aaron.
Flags: needinfo?(jmathies)
Flags: needinfo?(aklotz)
I'm to busy firefighting right now to look at this, sorry.
Flags: needinfo?(aklotz)
(In reply to Jorg K (GMT+1) from comment #19)
> Created attachment 8822226 [details]
> Send-to--Mail-recipient-crash.txt
> 
> With these two registry keys set:
> 
> [HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla Thunderbird]
> @="Mozilla Thunderbird"
> "DLLPath"="C:\\mozilla-source\\comm-central\\obj-x86_64-pc-
> mingw32\\mailnews\\mapi\\mapiDLL\\mozMapi32.dll"
> 
> [HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{29F458BE-8866-11D5-A3DD-
> 00B0D0F3BAA7}\LocalServer32]
> ""="\"C:\\mozilla-source\\comm-central\\obj-x86_64-pc-
> mingw32\\dist\\bin\\thunderbird.exe\" /MAPIStartup"
> 
> I get |Sent to > Mail recipient| to start my debug build and at times it
> crashes. Attached a stack.
> 
> So it crashes in nsAppShell::ProcessNextNativeEvent() inside
> ::DispatchMessageW(&msg):
> https://dxr.mozilla.org/mozilla-central/rev/
> d7b6af32811bddcec10a47d24bd455a1ec1836fc/widget/windows/nsAppShell.cpp#375
> 
> I have no idea how to debug this further, so let's see who can help.

There should be more to that stack unless the main thread is off servicing something in com.. and even then there should be something on the stack above dispatch.

Generally I know nothing about this mapi dll stuff. You want to track down someone who has worked with it. I haven't.
Flags: needinfo?(jmathies)
Assignee: nobody → wgianopoulos
Status: NEW → ASSIGNED

Comment 22

2 years ago
Thanks for taking the bug, I'm here to help (or pressure) you ;-)

Comment 23

2 years ago
Any progress here, Bill?
Assignee: wgianopoulos → nobody
Status: ASSIGNED → NEW
Aaron, can you look at this?
Flags: needinfo?(aklotz)
Not at the moment, sorry.
Flags: needinfo?(aklotz)

Comment 26

a year ago
I just did a send-to of a pdf with win explorer with 32-bit 52.6 on windows 7 with no problems. Also, from within OpenOffice Writer program I did a "send" function via tb of an odt file and it worked fine too. I'm not seeing any crash. I haven't done anything to the registry regarding this. 
I will try on win10 now. Not sure if tb there is 32 or 64 bit. If 32, will try to change it to 64.

Comment 27

a year ago
Works OK on win10 32-bit tb too. Tried to find the 64 bit tb but apparently there is currently only a 32-bit official version for win. I did not know that. Didn't see a 64-bit win release build in ftp either.
So tried the 64-bit daily version installer for tb. It runs OK but crashes when I do a send-to from win explorer. Can't tell why; no stack dumped that I see.

Updated

9 months ago
Summary: Can't "send-to" (MAPI) using Seamonkey and Thunderbird X86_64 → Can't "send-to" (MAPI) using Seamonkey and Thunderbird X86_64-bit

Comment 28

9 months ago
Today i migrated from 32Bit TB to 64bit on Win10-1803. Initially the "Send-to" was - apparently - not working but after i set Thunderbird as the default mail software in system settings it started to work without any problems.

I would recommend that, it it does not work in the first time or if Thunderbird is already set as the default mail software, set Windows Mail App as default and then set Thunderbird as default again. 

It will probably solve the case :)

Comment 29

9 months ago
Win10-1803 64Bit + Thunderbird 60beta9 64Bit

Thunderbird is set as default mail client within Windows settings.

Send-to works as expected without issues from Word documents, pdf files, websites (all tested browser like Firefox, Edge, Chrome...) and Firefox browsers "Email link..." from location bar.

Are there any 'official test cases' we can run to verify whether send-to performs correctly or not? Otherwise everything indicates "send-to" is ready to be marked as solved for Thunderbird 64Bit on Windows.

Comment 30

9 months ago
(In reply to Aris from comment #29)
> Send-to works as expected without issues from Word documents, pdf files,
> websites (all tested browser like Firefox, Edge, Chrome...) and Firefox
> browsers "Email link..." from location bar.
Thanks for testing. In my experience, it sometimes works, sometimes it doesn't. Sadly, as a TB developer, I have 5+ versions installed concurrently and it's even hard to determine which version will be invoked.

How about right-clicking a document on the desktop and selecting "Send to > Mail recipient". Or is that what you've already done?

Comment 31

9 months ago
I know what you mean. ;-)

So far I only tested Tb 52.9 32 Bit version and 60b9/10 64 Bit version on different machines and "Send to > Mail recipient" works fine, if Tb 60b9/10 64 Bit is set to be the default mail client for .eml/.wdseml/mailto files and protocols. 
W10 settings > Apps > default Apps > Email > Thunderbird
W10 settings > Apps > default Apps > "default settings per app" > Thunderbird > Manage > file types & protocols ( https://i.imgur.com/9h8clJg.png )

Comment 32

8 months ago
Can we close this bug as it does not seem to be valid anymore?
Well,

On Friday I installed the latest TB beta: 60 beta 10 x64.

When I right-clicked in a desktop file and selected to e-mail it, it exited TB.

It seems not to crash TB but it exits it.

Windows 10 Home, latest build.

Comment 34

8 months ago
Exiting *is* the crash.
If someone retests this and finds it working:

Make sure no x86 version of SeaMonkey or Thunderbird is installed. Uninstall all x86 versions completely and maybe use a registry cleaner in case some stale data is left. Depending on when you did install it one of the x86 mapi handler dlls might win. I use an old ad-free version of ccleaner for cleaning (up to 5.40 it is fine). 

With only the x64 version installed bot x64 TB and SeaMonkey just close when you use this feature.

Comment 36

8 months ago
I can reproduce the crash with 60 beta 10 x64 *now*, but it did not happen with beta 9 before beta 10 release/update.
(In reply to Aris from comment #36)
> I can reproduce the crash with 60 beta 10 x64 *now*, but it did not happen
> with beta 9 before beta 10 release/update.

Aris, Did you get a crash report?  If you did, pleas post the crash ID.
Flags: needinfo?(aris-addons)

Comment 38

8 months ago
This crash does not write any crash reports, sorry.
Flags: needinfo?(aris-addons)

Comment 39

8 months ago
Tested 60.0 x64 final version and "Send to" didn't work and Thunderbird crashed (with no crash report), even after setting .eml, .wdseml and MAILTO as default.

Got it working with above suggestions:
- Uninstall x86 Version
- Use CCleaner
- Set Thunderbird as default-handler for .wdseml (was missing)

Comment 40

7 months ago
I found exactly the same issue, although not on all the computers at my disposal.
I'll explain.

First of all: Thunderbird is setted as the default program to sent mail.

First computer (desktop pc) with Operating System Windows 10 Pro 64 bit (in Italian).
Thunderbird 60.0 64-bit (EN release): the "Send to" command does not work.
That is:
1) if Thunderbird is already open, at the moment you use the command (right click) "send to" the mail program is closed and no window for the crash report is presented; so, as already mentioned, the crash starts and ends with the closure of Thunderbird and, of course, the e-mail to be sent is not prepared;
2) if Thunderbird is still closed and you try anyway to send with the command (right click) "send to" any file, the e-mail is not packaged.

Attempted with the suggestion mentioned above (accurate uninstall of any debris of the previous 32-bit version, deep cleaning with CCleaner and assignment of Thunderbird as the default-handler for .wdseml), the issue remains.

Second computer (notebook) with Windows 10 Pro 64 bit Operating System (in English).
Thunderbird 60.0 64-bit (EN release): the "Send to" command works, although on very rare occasions it does not respond and, in any case, does not close the program when it does not execute the command (only it does not create the e-mail to send with the attachment).
So, for the most part, the option (right click on file or files) "send to mail recipient" responds as appropriate.
In the notebook the extension .wdseml does not appear, however, among the file types to be associated with some app.

I hope I have been clear enough and I trust in a support (and comfort)!

To recover the full and correct functionality of the "Send to mail recipient" command on the desktop computer (the first I mentioned) I reinstalled the 32-bit version of Thunderbird (EN release) and everything respond as expected and due.

Thank you!

Comment 41

6 months ago
Meh...
i - too - hit this bug again in a totally different computer and the only solution (after 3h of trying to fix it) was to also revert back to 32bit :(
(In reply to Wayne Mery (:wsmwk) from comment #4)
> per sid0 "the problem is probably that the xp64 explorer can't load our
> 32-bit proxy. the Vista explorer handles it by doing it out of proc (IIRC)"

Perhaps a different issue but with 64 bit Thunderbird on 64bit win7 I crash

odLoad: 000007fe`ea890000 000007fe`ea89b000   C:\Program Files\mozilla.org\TB 64.0a1 2018-09-20\MapiProxy_InUse.dll
Critical error detected c0000374
(5d98.1ea0): Break instruction exception - code 80000003 (first chance)
ntdll!RtlReportCriticalFailure+0x2f:
00000000`775cf2cf cc              int     3
Part of stacktrace analysis

GetUrlPageData2 (WinHttp) failed: 12002.
FAULTING_IP: 
ntdll!RtlReportCriticalFailure+2f
00000000`775cf2cf cc              int     3
EXCEPTION_RECORD:  ffffffffffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 00000000775cf2cf (ntdll!RtlReportCriticalFailure+0x000000000000002f)
   ExceptionCode: 80000003 (Break instruction exception)
  ExceptionFlags: 00000000
NumberParameters: 1
   Parameter[0]: 0000000000000000
CONTEXT:  0000000000000000 -- (.cxr 0x0;r)
rax=0000000000000000 rbx=00000000c0000374 rcx=000053a09d1d0000
rdx=000000000000fffd rsi=0000000000000000 rdi=0000000077637c70
rip=00000000775cf2cf rsp=000000000039de00 rbp=000000000e3538e0
 r8=0000000000000000  r9=00000000fffffff8 r10=0000000000000000
r11=000000000039d990 r12=0000000000000008 r13=000007fefdfa3e58
r14=000007feea892180 r15=000000000039e430
iopl=0         nv up ei pl nz na pe nc
cs=0033  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000200
ntdll!RtlReportCriticalFailure+0x2f:
00000000`775cf2cf cc              int     3
FAULTING_THREAD:  0000000000001ea0
PROCESS_NAME:  thunderbird.exe
ERROR_CODE: (NTSTATUS) 0x80000003 - {EXCEPTION}  Breakpoint  A breakpoint has been reached.
EXCEPTION_CODE: (HRESULT) 0x80000003 (2147483651) - One or more arguments are invalid
EXCEPTION_PARAMETER1:  0000000000000000
DETOURED_IMAGE: 1
NTGLOBALFLAG:  0
APPLICATION_VERIFIER_FLAGS:  0
CHKIMG_EXTENSION: !chkimg -lo 50 -d !KERNEL32
    773fa3f0-773fa3f6  7 bytes - KERNEL32!RegSetValueExW
	[ 4c 8b dc 49 89 5b 10:e9 33 5e bf f8 cc cc ]
    77403f00-77403f04  5 bytes - KERNEL32!RegQueryValueExW (+0x9b10)
	[ 4c 89 4c 24 20:e9 7b c2 be f8 ]
    774059c0-774059ca  11 bytes - KERNEL32!BaseThreadInitThunk (+0x1ac0)
	[ 48 83 ec 28 85 c9 75 0e:49 bb 40 37 9f fa fe 07 ]
    774059cc - KERNEL32!BaseThreadInitThunk+c (+0x0c)
	[ d2:e3 ]
    77409020-7740902c  13 bytes - KERNEL32!SetUnhandledExceptionFilter (+0x3654)
	[ 48 8b c4 48 89 58 18 55:49 bb a0 43 cc a9 fe 07 ]
    7741ffd0-7741ffd4  5 bytes - KERNEL32!RegDeleteValueW (+0x16fb0)
	[ 48 89 4c 24 08:e9 e3 01 bd f8 ]
    7742f3f0-7742f3f4  5 bytes - KERNEL32!K32GetMappedFileNameW (+0xf420)
	[ ff f3 55 56 57:e9 1b 0d bc f8 ]
    7743bac0-7743bacc  13 bytes - KERNEL32!RtlInstallFunctionTableCallbackStub (+0xc6d0)
	[ 48 83 ec 38 48 8b 44 24:49 bb 60 23 9f fa fe 07 ]
    77459c80-77459c86  7 bytes - KERNEL32!K32EnumProcessModulesEx (+0x1e1c0)
	[ 4c 8b dc 49 89 5b 10:e9 53 64 b9 f8 cc cc ]
    77469710-77469714  5 bytes - KERNEL32!K32GetModuleInformation (+0xfa90)
	[ 48 89 5c 24 08:e9 33 6a b8 f8 ]
    77488ac0-77488ac6  7 bytes - KERNEL32!RegSetValueExA (+0x1f3b0)
	[ 48 8b c4 44 89 48 20:e9 2b 77 b6 f8 cc cc ]
79 errors : !KERNEL32 (773fa3f0-77488ac6)
APP:  thunderbird.exe
ANALYSIS_VERSION: 6.3.9600.17298 (debuggers(dbg).141024-1500) amd64fre
LAST_CONTROL_TRANSFER:  from 00000000775cf8d6 to 00000000775cf2cf
ADDITIONAL_DEBUG_TEXT:  Followup set based on attribute [Is_ChosenCrashFollowupThread] from Frame:[0] on thread:[PSEUDO_THREAD]
BUGCHECK_STR:  APPLICATION_FAULT_MEMORY_CORRUPTION_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_LARGE
PRIMARY_PROBLEM_CLASS:  MEMORY_CORRUPTION_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_LARGE
DEFAULT_BUCKET_ID:  MEMORY_CORRUPTION_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_LARGE
STACK_TEXT:  
00000000`00000000 00000000`00000000 memory_corruption!KERNEL32+0x0
STACK_COMMAND:  .ecxr ; kb ; dt ntdll!LdrpLastDllInitializer BaseDllName ; dt ntdll!LdrpFailureData ; ** Pseudo Context ** ; kb
SYMBOL_STACK_INDEX:  0
SYMBOL_NAME:  memory_corruption!KERNEL32
FOLLOWUP_NAME:  MachineOwner
MODULE_NAME: memory_corruption
DEBUG_FLR_IMAGE_TIMESTAMP:  0
FAILURE_BUCKET_ID:  MEMORY_CORRUPTION_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_LARGE_80000003_memory_corruption!KERNEL32
BUCKET_ID:  X64_APPLICATION_FAULT_MEMORY_CORRUPTION_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_LARGE_DETOURED_memory_corruption!KERNEL32
IMAGE_NAME:  memory_corruption
ANALYSIS_SOURCE:  UM
FAILURE_ID_HASH_STRING:  um:memory_corruption_actionable_heap_corruption_heap_failure_block_not_busy_large_80000003_memory_corruption!kernel32
FAILURE_ID_HASH:  {b9007595-b747-e73a-92d5-47dd2b676e40}
hmm, just noticed comment 35. :(  Don't really have time to run it down.

Comment 45

4 months ago
No idea whether bug 1506488 would help. I'll check it when it's ready.

According to the information in this bug, for some users it sometimes works, and sometimes it doesn't. And several users had mentioned they might have a mix of 32bit and 64bit software installed. Then I saw some reports on the web, which claimed that the sendto isn't working on 64bit systems, or that the way mapi is working on recent windows systems has changed.

I suggest to begin analyzing this issue with a fresh 64-bit machine, with a thunderbird 64-bit the only version ever installed. We should learn if that works, or fails, and how often, and if necessary, add some printf statements for logging to a debug build.

I tried to start with that approach on Windows 10, but apparently it doesn't work at all.

I haven't used a TB installer. I have a local comm-central debug build. On start if offers to set it as the default mail client, and I accepted.

I used a web page with a <a href="mailto:..."> link, and when clicked from Firefox, I confirmed that the TB debug build is started, and the mail compose window opens.

Then I used file explorer, right click, send to mail recipient. I get an error, that no matching application is installed or configured.

I went back to the Windows default apps. I changed the preferred Mail application to the Windows Mail program. Attempted again to "send to", and still the same failure message is shown!

If you open the Windows SendTo folder (that's the place where the shortcuts for the send-to right click menu are managed), I learned that the "email recipient" file has a ".mapimail" extension.

In Windows "default apps", if you scroll down, there's an option to "choose default applications by file type". Click, and scroll down, until you see ".mapimail" on the left hand side. On my system, the right hand side is unconfigured, it offers "+ choose a default". When clicked, the only selection I'm offered is: "Look for an app in the app store".

I conclude, neither the default Windows Mail app, nor Thunderbird are registered with the system as appropriate destinations for this message type.

Could this explain why users have trouble with the 64-bit version of Thunderbird and the send-to feature?

Could the reports, that it sometimes works, be related to 32-bit applications still being registered somewhere in the system, and Windows still using them? (potentially because it works differently on the win32 subsystem)

I found several discussion threads with people not being able to use the send-to email feature. The only solutions I found, which were confirmed by some users, were about Outlook.

I suggest that someone researches how to make Thunderbird compatible with .mapimail, or make it compatible with it.

Additional experiment. In the SendTo folder (user/appdata/roaming/microsoft/windows/sendto) I double clicked the mail recipient file, and was offered to select an application. I navigated to the dist/bin directory of my local build and selected thunderbird.exe

This started thunderbird composer. When double clicking the mail recipient icon again, thunderbird starts again.

However, when using the file explorer to look at other folders, and use the right click menu to open the "send to" menu, the mail recipient option is gone!

Back to the default programs, choose by file type, the entry .mapimail now has an assignment to "thunderbird daily". So it's configured, but for some reason Windows believes it's incompatible, and removes it from the sendto menu???

Before we can consider a bug in Thunderbird, we need a learn about reliable way to configure this on a fresh system.

Comment 48

3 months ago

Umm, you must run an installer. To make one: mach pacakge. If you use Windows, "Everything" desktop search is a great little tool. It will help you to locate
... obj-x86_64-pc-mingw32\dist\install\sea\thunderbird-66.0a1.en-US.win64.installer.exe

For the default registration see bug 1509918 and bug 1517955.

Comment 49

3 months ago

Well, mach package.

Jörg, thanks for clarifying that the integration performed by the Thunderbird executable is insufficient, and thanks for pointing me to the installer.

After running the installer, "file explorer right-click send-to mail" no longer complains "no app", but it runs Thunderbird.

After playing with it for a while, I've observed:

If you skip running thunderbird immediately (disable checkbox at the end of the installer), and if you don't run thunderbird manually, but if "right-click send-to" is used as the very first action, then it doesn't work.

I could reproduce that with my debug build and my local installer. Because the debug binary is slow, when using send-to, I can see that a thunderbird process is being started (the debug build opens a terminal window, and I can see some trace statements).

This window closes after a few seconds, without bringing up any Thunderbird window. Using the thunderbird debug build, I can reproduce this multiple times.

Now, I started the thunderbird program, and waited until it opens up. During the first execution, it might be slow, and show the "spinning mouse cursor" for a longer amount of time. If I attempted to use "send-to" very quickly after starting thunderbird, I saw it once or twice, that the main thunderbird window closed (crashed?). But not always, sometimes, the compose window open fine, even if I clicked it quickly.

However, as soon as I've allowed thunderbird (after installation) to open, and have given it sufficient time (10-20 seconds), the right-click send-to mail ALWAYS works for me. After it worked once, it never failed for me again. Even after a reboot, if I don't start thunderbird yet, but use the right-click send-to mail as the first thunderbird action since boot, it works fine, and both the compose window and the thunderbird come up, and work.

If I uninstall, reboot, install again, I can again reproduce the behavior described above.

This could mean that Thunderbird triggers some system registration in the background. Only after that registration has succeeded once, it works reliably.

In addition to the above tests, which used my own debug binary and installer, I've also test using a download 60.4 release.

I want to note that the default download site sent me to a 32-bit Thunderbird installer, despite me using a Firefox 64 bit on a 64 bit windows. Initially, I didn't notice and installed the 32-bit build. With that, again I didn't allow the installer to run thunderbird immediately, and I used the right-click send-to mail as the first action. That immediately worked. So maybe the 32-bit version is indeed better.

I uninstalled 32bit, rebooted, manually searched for the 64-bit release, installed. Skipped launching ob thunderbird by installer. Use right-click send-to as first action. And it failed. Nothing happened.

Then, I immediately tried right-click send-to again. Now it worked.

So, my theory from above, that it needs a full start of thunderbird before it works, isn't true for the release binary.

Again, from this point, it always worked. I rebooted, and send-to as first action still worked.

My summary of the above is:

  • I can reproduce a send-to failure immediately after install
  • I cannot reproduce a failure as soon as thunderbird has been allowed to fully start up once

This brings up two questions:

(a)
Can anyone reproduce the failure more reliably?
After an install has worked for you once, do you ever see it nonworking?
(Please consider that each time after you have updated, you might allow it to start fully, before send-to works.)

If yes, then you have a problem that I cannot reproduce yet.

(b)
If the answer to (a) is "no", in other words, if some kind of (small) delay is necessary, until a new install works fine, is it worth to to spend more time to debug this further?

Further analysis that could be made are:

  • does the windows registry look differently before "first attempt with failure" and "once it works"?
  • same question for thunderbird profile?
    (if differences are found, could these changes be responsible for the occassional initial failure, and could we apply them immediately?)

If no changes are found, we should do some more experiments, to investigate if there's any time delay involved. If we wait for a longer time after the installer, will the first send-to attempt work immediately? If yes, this could mean some delay inside the windows registration stuff, which we might not be able to influence (this is just a wild guess, but it might be useful to rule this possibility out).

IMHO, only if the answer to (a) was "yes", or if the suggested investigations for (b) produces no solution, then we could spend more time on debugging this at the code level. It would require to identify the code pathes that are triggered by the right-click send to, and add some printf style tracing to a logfile. Probably two processes are involved, so we'd have to log to a file that includes the process id in its logfile. I'm guessing that it doesn't "crash", because crashes should trigger some crash reporter, right? Rather, I guess that some assumptions/expectations are not met, and the code decides to exit.

Comment 51

2 months ago

Hmm, yes, if you run TB, is does some system integration stuff (badly these days), you can see a discussion in bug 1509918.

Since x64 versions are not fully supported, we don't advertise them, but you can still get them, for example from here:
http://ftp.mozilla.org/pub/thunderbird/releases/60.4.0/win64/

So you didn't see the crash? Try this: Have TB running and then do the "Send to > Mail recipient". It takes down the running TB :-( - The bug is getting (too) long, but the crash is there, see comment #19 and corresponding attachment. You can even get a local build to crash.

Comment 52

2 months ago

Confirmed in comment #33. No "exit", it's a crash :-(

(In reply to Jorg K (GMT+1) from comment #51)

So you didn't see the crash? Try this: Have TB running and then do the "Send to > Mail recipient". It takes down the running TB :-( -

No crash for me. Windows 10 64bit, Thunderbird 60.4.0 64bit release installed.

Thunderbird is running, click on file in file explorer, send-to mail, new compose windows open in thunderbird, no crash.

Now! I can reproduce. Using my local debug build. As you said. Thunderbird exited three times in a row, on each attempt.

Comment 55

2 months ago

Just when I was gearing up to reproduce it ;-)

Yep, tried just now using TB 60.4 x64 bit. Running app just died :-(

I was able to reproduce it in the debugger.

I understand the bug. It's a memory cleanup issue.

When send-to is used, Windows loads the mozMapi32.dll, calls the MAPISendMail callback, and gives it memory that contains information about the requested message.

mozMapi32.dll then uses inter-process-communication to forward the request to the thunderbird process.

At the time we crash, thunderbird has completed executing the CMapiImp::SendMail function (which implements the NSIMapi.SendMail MS-COM interface). Then the OS runtime libraries attempt to free the memory received through the IPC interface. This crashes.

The reason is that the initial mozMapi32.dll simply forwards the memory that it has received from Windows. But Windows it allowed to free that memory, after the initial process is done sending the message to thunderbird.

The solution is to change mozMapi32.dll to copy the memory it has received by Windows, pass it with IPC to thunderbird, and don't free it. The pointers are given to thunderbird, and the OS runtime libraries will free them.

I have a fix in hand, which fixes the crash for me. I need to spend some more time to clean it up, and handle all possible scenarios, before I can attach it.

Also, I found a misdefined type in msgMapi.idl, which I need to clean up, too.

Comment 57

2 months ago

Great. Why doesn't it crash on x86 though? Pure luck?

Difficult to say. Maybe luck, maybe the order of events is different on x86. If mozMapi32.dll stays alive on x86 until after the CMapiImp::SendMail is completed, the freeing inside thunderbird succeeds, and the inability to free in the calling dll is ignored? This is just wild guessing. But duplicating the memory and passing it over should be safe on all architectures.

Posted patch 393302-v1.patch (obsolete) — Splinter Review

Jörg, does this patch work for you?

FYI, the existing code registers the MAPISendMail callback. That API uses parameters which are defined by the Windows/MAPI SDK.

In particular, parameter lpMessage is defined here:
https://docs.microsoft.com/en-us/windows/desktop/api/mapi/ns-mapi-mapimessage

Our code attempts to emulate this nested data type, see nsMAPI.idl, using the MS-COM IDL rules.

It has the nested element lpFiles, with the type defined here:
https://docs.microsoft.com/en-us/windows/desktop/api/mapi/ns-mapi-mapifiledesc

It uses LPSTR for path and file names.

However, our existing code deviates, and use type LPTSTR instead.
This is a mistake. We must use the types defined in the API.

While there is a W version of the APIs, which uses unicode and LPTSTR, we cannot mix the types, it must be either all without W or all with W.

Despite not using the W versions, sending a file from a local path containing Unicode characters, sending such files works fine for me on Windows 10. I'm guessing it uses UTF-8 for transport, which works with LPSTR.

I don't know the background of using LPTSTR in MapiFileDesc in the Thunderbird code. Maybe it was a mistake. Maybe it was an attempted hack to fix Unicode paths on some Windows versions? But I cannot imagine how changing the type could have helped, because the API as defined by the SDK defines what Windows will put into the parameters, and changing the type of the transport structure doesn't change what we receive.

Assignee: nobody → kaie

Updated

2 months ago
OS: All → Windows

kaie, where have you been these 12 years? :)

Thanks for touching this code. WRT LPSTR, this may not be terribly insightful but the decision perhaps was made in the simple mapi days, eg bug 108275. Note also, there was an effort to change simple mapi to LPSTR in bug 594239

Comment 61

2 months ago

I'll try the patch some other day/night.

Some remarks. Please do not use tabs. Or indentation is two spaces, although some old code is inconsistent. There are also no spaces between function names and (, so foo(x). We typically fix these things in the vicinity, like here in
hr = pNsMapi->SendMail ( ....

I've last tweaked this code in bug 1505315: https://hg.mozilla.org/comm-central/rev/e1449ad9e4d6

There was also an attempt to implement the MAPISendMailW interface in bug 1506488.

I think it makes no difference whether we use LPSTR or LPTSTR as long a the content is treated as char. But there's no harm in correcting it.

Finally I don't know what you mean by "local path containing unicode characters". Windows uses its own code pages, like windows1252 for most "Western" installation, or cp932 for Japanese. There is also UTF-8 as experimental system charset in Windows 10 ([ ] Beta: Use Unicode UTF-8 for worldwide language support"). Have you tried a filename with some CJK in it? You could see what arrives at the interface. NS_CopyNativeToUnicode() would assume the local code page as input, not UTF-8.

I'm not sure that the Unicode path that you're removing isn't triggered, see:
https://searchfox.org/comm-central/rev/f383ce2e20ebab18bc89bb305a3d6a1349020f3a/mailnews/mapi/mapihook/src/msgMapiImp.cpp#219
In fact, if that were never triggered, we could remove one of the PopulateCompFields* functions, no?

Comment 62

2 months ago

Bug 594239 is certainly interesting ;-)

Comment on attachment 9036164 [details] [diff] [review]
393302-v1.patch

Tested with SeaMonkey 2.53 x64 and no longer crashes under Server 2016. Mail window with attachment is created in the background. But the z-order problem is either something SeaMonkey specific or present since ages too.

Great job.

Attachment #9036164 - Flags: feedback+

(In reply to Jorg K (GMT+1) from comment #61)

Some remarks. Please do not use tabs. Or indentation is two spaces, although some old code is inconsistent.

Agreed, mixing tabs and spaces is painful. The editor inside the Windows VM wasn't setup correctly when I started writing the code. I'll fix this.

There are also no spaces between function names and (, so foo(x). We typically fix these things in the vicinity, like here in
hr = pNsMapi->SendMail ( ....

Besides tab/space mixing, I'm don't mind whitespace details much. I'm ok to fix the places that you point out and don't like, but I have a slight preference to avoid whitespace changes, as they can make a patch unreadable quickly (and make it more difficult to see where the relevant changes are).

Finally I don't know what you mean by "local path containing unicode characters".

I wanted to ensure that this patch doesn't make it worse regarding those characters. Yes, I had test a directory name consisting of japaneses characters, and it worked.

I'm not sure that the Unicode path that you're removing isn't triggered, see:
https://searchfox.org/comm-central/rev/f383ce2e20ebab18bc89bb305a3d6a1349020f3a/mailnews/mapi/mapihook/src/msgMapiImp.cpp#219

I'm not removing this code you're pointing to.
With the change of type LPTSTR to LPSTR, we muse use set aIsUnicode to false in function HandleAttachments. After this change, all calls to HandleAttachments use parameter false, so the parameter can be removed, and the logic for "true" can be removed in this function, because it's never used.

Posted patch (wrong file) (obsolete) — Splinter Review

v2 fixes whitespace, no other changes

Posted patch 393302-v2.patch (obsolete) — Splinter Review
Attachment #9036206 - Attachment is obsolete: true

Updated

2 months ago
Attachment #9036206 - Attachment description: 393302-v1.patch → (wrong file)

Comment 67

2 months ago

Thanks. I agree, too many white-space changes can be confusing, but we can certain fix issues on lines which are already touched, like:

+        memset (Recipients, 0, sizeof (nsMapiRecipDesc) );
should be
+        memset(Recipients, 0, sizeof(nsMapiRecipDesc));
+        memset (Files, 0, sizeof (nsMapiFileDesc) );
+  rv = HandleAttachments (aCompFields, aMessage->nFileCount, aMessage->lpFiles) ;  - 2 times
+            pFile->InitWithNativePath (nsDependentCString(aFiles[i].lpszPathName));
and more.

I know you're not removing the code I pointed out, but I'm asking myself whether we should do a broader clean-up here and remove all dead or non-functioning code including nsMapiHook::PopulateCompFields() which I doubt will be triggered or work. Refer to attachment 473834 [details] [diff] [review] of bug 594239 which did remove the code I pointed out.

Comment 68

2 months ago

JFTR:
MAPISendMailW

(In reply to Kai Engert (:kaie:) from comment #59)

Despite not using the W versions, sending a file from a local path containing Unicode characters, sending such files works fine for me on Windows 10. I'm guessing it uses UTF-8 for transport, which works with LPSTR.

MapiMessage structure
| MapiMessage structure:
[...]
| ulReserved
|
| Reserved; must be zero or CP_UTF8. If CP_UTF8, the following are UTF-8
| instead of ANSI strings: lpszSubject, lpszNoteText, lpszMessageType,
| lpszDateReceived, lpszConversationID.

vs.

MapiMessageW structure
| ulReserved

| Reserved; must be zero.

Comment 69

2 months ago

Comment on attachment 9036207 [details] [diff] [review]
393302-v2.patch

With these registry settings
[HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}\LocalServer32]
""=""C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\thunderbird.exe" /MAPIStartup"

[HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla Thunderbird]
@="Mozilla Thunderbird"
"DLLPath"="C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\comm\mailnews\mapi\mapiDLL\mozMapi32.dll"

Window's "Sent to > Mail recipient" will start the local build, no installer required (see comment #19 from two years ago.

Without the patch, my x64 build crashes, with the patch, it works :-)

I tried a filename with non-ASCII characters but within the local code page of windows-1252 (äö€ß.txt) and also a filename with Japanese (テスト.txt) and that doesn't work at all. TB isn't even launched as far as I can see. Most likely, whatever representation Windows uses for the filename internally, it detects that Japanses cannot be converted to the local ANSI code-page and refuses to work. That's good news since internally we now use NS_CopyNativeToUnicode() and that would mess up completely if UTF-8 were interpreted as native ANSI. Of course, say, Polish users will use windows-1250 and it will work for them. For me a file with many "Polish" characters (ąęćżźłóńś.txt) launches TB but doesn't launch the compose window, so that's inconsistent with Japanese. For those cases to work, we'd need to implement MAPISendMailW (bug 1506488). I also tried sending files with ASCII-names from folders called テスト and ąęćżźłóńś. In the former case, nothing happens, in the latter case TB starts but with no compose window.

So Kai, please let us know how you want to proceed here. I see these options:

  1. Fix the white-space nits and get it landed as it is. I can fix those nits to avoid further noise.
  2. We also remove the dead/non-functioning code I pointed out in comment #67, or I can do that in bug 594239.

For full unicode support we need bug 1506488.

Anyway, thanks for picking this up and finding the root cause of the problem. It looks like it needed a fresh pair of eyes since everyone else was busily trying to find out why 32bit worked and 64bit didn't when apparently 32bit only worked by luck.

Attachment #9036207 - Flags: review+

Comment 70

2 months ago

Addition: Switching the Windows system locale to UTF-8, see comment #61, regional settings, [ ] Beta: Use Unicode UTF-8 for worldwide language support, makes the テスト.txt and ąęćżźłóńś.txt files work as well :-) - So MS did get something right after all ;-)

I guess Kai used this setting in his VM since he reported Japanese filenames to be working. Surely if NS_CopyNativeToUnicode() detects UTF-8 as local code page, it will do the right thing.

(In reply to Jorg K (GMT+1) from comment #70)

I guess Kai used this setting in his VM since he reported Japanese filenames to be working.

FYI, in my VM it's not checked.

Jörg, thanks for the explanations.

(In reply to Jorg K (GMT+1) from comment #69)

So Kai, please let us know how you want to proceed here. I see these options:

  1. Fix the white-space nits and get it landed as it is. I can fix those nits to avoid further noise.

Thanks, I accept your offer.

(I haven't looked at the other cleanup suggestions. Handling it separately sounds good.)

Comment 73

2 months ago

OK, I'll get this landed and backported.

Target Milestone: --- → Thunderbird 66.0

Updated

2 months ago
Attachment #9036164 - Attachment is obsolete: true

Updated

2 months ago
Attachment #9036207 - Flags: approval-comm-esr60+
Attachment #9036207 - Flags: approval-comm-beta+

Updated

2 months ago
Status: NEW → ASSIGNED

Comment 74

2 months ago

(In reply to Kai Engert (:kaie:) from comment #71)

FYI, in my VM it's not checked.

Hmm, hard to tell what's going on there. The underlying Linux file system surely uses UTF-8 names, so maybe it's all UTF-8 under the cover even without checking the box. Somehow the VM maps the local file system into the VM's file system. What I observed on a real system certainly makes sense. Anyway, the entire encoding discussion is separate from fixing the crash by copying the data and correcting the types. And with that, we should be very happy since it was the blocker for the x64 roll-out on Windows.

As anxious as we may be to ship this to users, I think this should have a good settle time in beta before hitting esr

Comment 76

2 months ago

Here's the patch with the white-space issues fixed. I've also added checks after each call to CoTaskMemAlloc. Use interdiff to see the changes.

Attachment #9036358 - Flags: review+
Attachment #9036358 - Flags: approval-comm-esr60+
Attachment #9036358 - Flags: approval-comm-beta+

Updated

2 months ago
Attachment #9036207 - Attachment is obsolete: true
Attachment #9036207 - Flags: approval-comm-esr60+
Attachment #9036207 - Flags: approval-comm-beta+

Comment 77

2 months ago

Comment on attachment 9036358 [details] [diff] [review]
Kai's 393302-v2.patch, plus Jörg's whitespace cleanup and more memory checking.

I've looked at this for a while now and I've come up with some questions and noticed some omissions:

  1. Can you clarify comment #56: "The pointers are given to thunderbird, and the OS runtime libraries will free them." - The way I understand it, Windows interfaces with MAPISendMail in MapiDll.cpp and that somehow calls into a TB processes (via IPC) and the SendMail() function. What is the process of freeing the memory we allocate?
  2. Do you understand why the SendMail() interface passes the message and the counts of recipients and file attachments? Why can't the message structure hold those pointers and the interface be reduced to just passing the message? These assignment on the receiving end aMessage->lpRecips = aRecips; appear strange when the could be done in the caller. Hint: I tried doing what I just said, and it doesn't work any more, so there must be some reason.
  3. I think you forgot to populate flFlags in the file copy, as well as ulRecipClass in the recipients and flFlags and lpOriginator in the message itself. Whilst these fields might not matter for the "Sent to > Mail recipient" function, they would matter in a full MAPI implementation, do you agree?

Please start further work with the patch I attached.

Flags: needinfo?(kaie)
Attachment #9036358 - Flags: review+
Attachment #9036358 - Flags: approval-comm-esr60+
Attachment #9036358 - Flags: approval-comm-beta+

Comment 78

2 months ago
Posted patch API experiment 1 (obsolete) — Splinter Review

Here's my attempt to simplify the interface, and it doesn't work :-(

Comment 79

2 months ago
Posted patch API experiment 2 (obsolete) — Splinter Review

I tried to understand how the invocation mechanism works and whether there's anything special about the SendMail() interface. In this patch I've renamed it, I'm pretending that I'm passing one file and I'm setting the message in MapiDll.cpp. It's still working, and one can create a message with more than one file. As soon as you change the interface like I did in the other patch, things stop working. Windows must somehow know about that interface since in one experiment I passed one file with an allocated but empty structure in it seemed to crash Windows Explorer. I'm not really doing this for fun, I want to work out how it hangs together and whether we really don't need to free what we allocate, and also, why we need the extra parameters when we can pass everything in the message structure.

Comment 80

2 months ago

User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0 SeaMonkey/2.53
Build-Identifikator: 20190114130007

Works for me. Great job.

Jörg, if you want to make more changes, we'll have to invest much more time in this and learn in more detail how MS-COM works. Does this part of the code deserve that much attention?

The inter-process communication here apparently involves serialization and deserialization of data. If it works when having the lists (recip and files) as separate parameters in the IDL, then possibly Windows will use the instructions in the function signature of the IDL to correctly handle them. You said it doesn't work if the lists are transfered inside nsMapiMessage. So apparently, despite the nested type nsMapiMessage also using statements like size_is (nRecipCount), it doesn't work. In order to understand why, we'd have to become experts for MS-COM message exchange.

The code in MapiDll.cpp contains the following comment:
// The MS COM type lib code generated by MIDL for the MS COM interfaces checks for these parameters
// to be non null, although null is a valid value for them here.

Because of that comment, I allocate an empty structure for the lpRecips parameter and memset it to zero.

However, when I was debugging on the receiving side inside CMapiImp::SendMail, parameter aRecips pointed to random memory, not filled with zero. That's fine, because we ignore it, as aRecipCount is zero.

Apparently the Windows MS-COM IPC code decided it doesn't matter what aRecips is set to, because aRecipCount is zero.

It also shows that CMapiImp::SendMail doesn't receive the direct values that were originally given to the SendMail call inside MapiDll.cpp, but some transformation is in between.

I'm suggesting that we do the minimal amount of changes to fix this bug, because apparently, the code mostly works. The knowledge the original authors had on this, might have been broader than what we have currently. We shouldn't make more, unnecessary changes, unless we're willing to learn about how it all works. I'd like to avoid an trial and error approach.

The original bug was clearly a memory issue, because it crashed inside NdrOleFree, a wrapper for CoTaskMemFree. So obviously incorrectly allocated, or incorrectly owned memory is involved. That's all that's really obvious, without being an expert on MS-COM IPC.

Flags: needinfo?(kaie)

The correct way to construct and manage the memory exchanged, might be to use functions like CoCreateInstance to instantiate the data we want to send, use AddRef() on the calling side, and use Release() on the receiving side, once we're done. I don't know how close the existing code is to use that approach. As I said, if we want to go the route to make it completely correct, we'll need to learn and spend much more time on this, and it might result in a bigger rewrite.

https://docs.microsoft.com/en-us/windows/desktop/api/combaseapi/nf-combaseapi-cocreateinstance
https://docs.microsoft.com/en-us/previous-versions/ms810016(v=msdn.10)

Comment 83

2 months ago

Well, I've looked through the MAPI code, and it's really in a deplorable state, I'll file a new bug for it. Here some examples:

MapiDll.cpp: SetPointerArray() never used, hence, memArray never written, hence MAPIFreeBuffer() most likely doing nothing. MAPISaveMail() doing a bunch of stuff, then returning MAPI_E_FAILURE unconditionally.

As for your question: Does this part of the code deserve that much attention?. MAPI is the official interface where Windows communicates with the e-mail client, this interface consists of various functions. We have no knowledge which functions actually work and which ones don't, and worse, and we have no knowledge which function are used by other Windows software, like MS Office or LibreOffice, interfacing with TB through MAPI. All we have looked at in this patch is where Windows itself interfaces with TB through "Send to > Mail recipient". In this case, as debugging showed, there is no recipient and we only get a bunch of files.

I'm about 100% sure now that your patch breaks the MAPISendMail() for other applications, for the simple fact mentioned in comment #77 point 3. You don't copy over various fields which are used at the receiving side, I've checked the code. So if some Office or scheduling software tries to send a message with To, CC and BCC, and originator set, we've just broken that. So despite the joy of having the issue fixed, the patch cannot land in its current form.

In general, fixing software we don't understand, is very treacherous ground. I can understand that we've run into some memory ownership problems here which you fixed by taking (so far incomplete) copies. I'm not convinced that this memory isn't leaked. We would have to make the conscious decision to accept that.

As you correctly point out, some transformation happens between MapiDll.cpp and CMapiImp::SendMail(), as my investigation patch shows, the message is passed intact, so I don't understand why we need to pass recipients and files separately. You can pass one file, and if the message specifies two, two get attached.

I think key to a fix that will hold water in the medium term is to understand who the passing works and who is responsible for freeing the memory.

At the very least, to move this patch forward, we need to copy over all the fields I mentioned, and personally I've like to assign the recipients and files into the message structure in MapiDll.cpp and not at the receiving end. It would be great to understand why that CMapiImp::SendMail() interface apparently cannot be changed since MAPISendMail() does not have separate arguments for recipients and files.

Masatoshi-san, do you know?

Comment 84

2 months ago

One more thing: I know from other bugs that MAPI is used by all sorts of weird 3rd party applications, not only the mentioned office packages. So the very least we need to do is to maintain it in working state.

Updated

2 months ago
Flags: needinfo?(VYV03354)

I agree that we should copy over the integer flags. Yes, the suggested patch might potentially leak. If we want to ensure this is avoided, we'll probably have to instantiate COM objects, and use IUnknown reference counting (not XPCOM).

If you're certain the suggested workaround has negative side effects, the patch should probably be marked as r- or obsolete.

Comment 86

2 months ago

I don't know what the way forward here is. At the very least, we need to copy over the integer flags and the lpOriginator.

You mentioned the risk of potential regression. Do you know how to test and confirm that a new patch don't introduce
MAPI regressions?

Jörg, it's your call if you accept a minimal patch that potentially leaks and doesn't implement your mentioned API change.

Perhaps Mike can offer some advice (but he hasn't been around in a while)

Flags: needinfo?(mikekaganski)

Updated

2 months ago
Duplicate of this bug: 1520135

Comment 90

2 months ago

(In reply to Kai Engert (:kaie:) from comment #87)

You mentioned the risk of potential regression. Do you know how to test and confirm that a new patch don't introduce
MAPI regressions?

Jörg, it's your call if you accept a minimal patch that potentially leaks and doesn't implement your mentioned API change.

At the very least, we need to copy over the mentioned fields. Any objections to do that? Then we can try at least LibreOffice to send.

Further before we ship this in TB 60.x, we should involve the reporters and commenters from these bugs: bug 1367141 (using SOCMAPI.EXE to create MAPI emails from our custom Helpdesk software in Thunderbird for years), bug 1370814 and bug 1366196.

That "helpdesk software" might pump out lots of messages and leaking memory may be an issue, I can't tell.

Personal note: While bug 1367141 was a WONTFIX, I've written an add-on for these people and the ones from bug 1367716, so they could keep their workflow.

(Assignee)

Comment 91

2 months ago

Hi! :-) Nice to meet you.

@Wayne: I'm sorry, tried to briefly follow the thread, but it's quite lengthy... could you brief me wrt what my advise should be about?

By the way, as a LibreOffice developer working on Windows x64 and using TB, I can say that personally I don't see any problems (except for lack of Unicode MAPI which I had prepared a patch for, but which didn't get any review so far ;-))

Comment 92

2 months ago

BTW, with the patch, I can send a document from LibreOffice, but it's the "no recipient, single file case" as well.

Hi Mike, long time, no see ;-) - You're working for LibreOffice now. Do you have knowledge of MS-COM? What do you think of the patch? And were is your patch awaiting review? Oh, for LibreOffice, not TB?

(Assignee)

Comment 93

2 months ago

Hi Jorg! o/

Well - wrt COM - I need to re-learn things every time I need to touch it ;-)
But which patch exactly is being discussed - I see some obsolete, and some not...

The patch I referred to is for bug 1048658 (/me doesn't loose a chance to advertise own patches when possible, in the hope nobody notices the offtopick :-P)

FYI, I found a sample application for Aimple MAPI here:
https://support.microsoft.com/en-us/help/171096/simple-mapi-console-application

Comment on attachment 9036387 [details] [diff] [review]
API experiment 1

Better description for this attachment

Attachment #9036387 - Attachment description: 393302-follow-up.patch → API experiment 1

Comment on attachment 9036440 [details] [diff] [review]
API experiment 2

Better description for this attachment

Attachment #9036440 - Attachment description: investigation.patch → API experiment 2

Comment on attachment 9036358 [details] [diff] [review]
Kai's 393302-v2.patch, plus Jörg's whitespace cleanup and more memory checking.

Renaming this patch as well.

I intend to attach a further enhancement of this patch, which copies the additional fields that Jörg mentioned.

Attachment #9036358 - Attachment description: 393302-v2.patch (v3) → Kai's 393302-v2.patch, plus Jörg's whitespace cleanup and more memory checking.

Comment 98

2 months ago

(In reply to Masatoshi Kimura [:emk] from comment #94)

FYI, I found a sample application for Aimple MAPI here:
https://support.microsoft.com/en-us/help/171096/simple-mapi-console-application

Interesting. Yes, that exercised the various MAPI functions without telling us how to implement them on the other side. Could be good for testing though.

(In reply to Mike Kaganski from comment #93)

Well - wrt COM - I need to re-learn things every time I need to touch it ;-)
But which patch exactly is being discussed - I see some obsolete, and some not...

(v3) is what we're trying to land with some additions. I'd like to understand why the "follow-up" on top of that breaks things. And the investigation patch tweaks (v3) so it still works.

Comment 99

2 months ago

Oops, they've been renamed, so experiment 1 doesn't work, but experiment 2 does.

Posted patch 393302-v4.patch (obsolete) — Splinter Review

This copies the additional fields. I've moved the redundant copy code to local (static) helper funtions.

Attachment #9036358 - Attachment is obsolete: true

Comment 101

2 months ago

Comment on attachment 9036603 [details] [diff] [review]
393302-v4.patch

This is still working ;-) - and copying all the fields, so we don't lose functionality. Oh, great clean-up of the ugly duplication.

I'll see whether Mike has some input, he used to be at GMT+10 or so. Otherwise I'm happy to go with this for now.

Attachment #9036603 - Flags: feedback+

Comment 102

2 months ago

Comment on attachment 9036387 [details] [diff] [review]
API experiment 1

Mike, this used to apply on top of (v3), but you get the idea. Any idea why this patch breaks the functionality completely? Where are the arguments of SendMail() set in stone? Also please take a look "experiment 2" which doesn't change the function, but transports the information inside the message object.

Flags: needinfo?(mikekaganski)
Attachment #9036387 - Flags: feedback?(mikekaganski)

Updated

2 months ago
Attachment #9036603 - Flags: feedback?(mikekaganski)
(Assignee)

Comment 103

2 months ago
Comment on attachment 9036603 [details] [diff] [review]
393302-v4.patch

Review of attachment 9036603 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/mapi/mapiDll/MapiDll.cpp
@@ +259,5 @@
>  
> +    if (!allocAndCopy(&Message->lpszSubject, lpMessage->lpszSubject))
> +        return MAPI_E_INSUFFICIENT_MEMORY;
> +    if (!allocAndCopy(&Message->lpszNoteText, lpMessage->lpszNoteText))
> +        return MAPI_E_INSUFFICIENT_MEMORY;

So this and following "return-on-error" lines would leak the previously allocated memory? I don't know if the process would terminate then (i.e., if the DLL is loaded into a process which only does a single job and exits, or if it exits seeing the function failing) which would make this OK, but as you were concerned about memory management in the issue thread, I suppose there should be some kind of raii to handle this properly.

Comment 104

2 months ago

I guess we're not so worried about memory management here, more about fixing the crash. It's highly unlikely that we run out of memory. As far as I can see, all the memory allocated here to create a clone of the message is leaked anyway, at least we don't understand the mechanism of it being free'd.

The feedback we're seeking from you is what you think of the approach in general. Since the 32bit binaries don't crash, the memory passing between the MAPI DLL and the callee in TB must be different for 64bit. I was dreaming of a one-line fix where we tell Windows to handle the 64bit case the same as the 32bit case so no further changes would be necessary here.

So right now, we either go with the proposed solution of making a copy, mismanaging it a bit if some later allocation fails, and then leaking it all in the end ;-(

(Assignee)

Comment 105

2 months ago

Sigh. I need to setup TB development environment. Sorry for the silly question (I try to save a couple of search cycles): could you give a direct link to the up-to-date manual how to setup it?

Comment 106

2 months ago

I'll need that anyway for the MAPISendMailW bug ;-) - Note comment #69 (registry entries) to get your local build to handle the MAPI requests. Other than that, it's still the same instructions:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build
Also the "Windows Build Prerequisites" on that page.

This also might be worth a read:
http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2018-April/001152.html
Ignore the P.S., the build instructs were brought up-to-date.

(In reply to Jorg K (GMT+1) from comment #104)

The feedback we're seeking from you is what you think of the approach in general. Since the 32bit binaries don't crash, the memory passing between the MAPI DLL and the callee in TB must be different for 64bit.

It could be a timing issue, and a different order of cleanup routines. For example, potentially the 32-bit MAPI/MS-COM routine could block until its done, while on 64-bit potentially it could be implemented inside Windows fully asynchronously. This could potentially cause the MS-COM cleanup inside the 64-bit thunderbird process to happen earlier than in the 32-bit.

Mike, our suggested hack allocates new memory, which we pass on into MS-COM, and never free it, in the hope it avoids all crashes.

I suspect the correct fix would be to instantiate real MS-COM objects, and use MS-COM reference counting.

But we currently haven't learned how to do that correctly.

Currently, API nsIMAPI.SendMail passes an object of type nsMapiMessage. How would we correctly instantiate an MS-COM object of that type, allowing us to use AddRef() and Release() on it? (Which probably would avoid the crash and wouldn't leak.)

I believe the original implementor of the code didn't know that (and didn't know about the need to use reference counting on it, because the old code simply forwarded the same pointer, as received from windows, into the remote MS-COM call nsIMAPI.SendMail.)

But the original implementor had apparently noticed issues when forwarding nested arrays inside nsMapiMessage, when forwarding it with the nsIMAPI.SendMail call. That's why the original implementor had chosen to move the nested arrays (lpRecips and lpFiles) to parameters of the nsIMAPI.SendMail call.

(Jörg's experimental patch attempted to remove the separate parameters from the call, and instead move them into the structure - which didn't work.)

Jörg's thought is, can we avoid learning all the MS-COM object handling details ourselves, and rely on your experience to help us out?

(Assignee)

Comment 109

2 months ago

Is there something missing in the "Building Firefox for Windows" referenced from "Simple Thunderbird build" with a note "To build on Windows, please read the prerequisites so you don't skip preparing the MAPI headers needed to compile Thunderbird"? I don't see anything there mentioning MAPI, and I had got missing MAPI headers errors. The directions for MAPI are down there on the "Simple TB Build", so maybe move them above to replace the warning?

(Assignee)

Comment 110

2 months ago

I have built TB now (since I started my work for LO, I always admire how fast TB is built) - but it turned out to be 32-bit. I see some mentions of mozconfigs for 64-bit, but they all lack anything 64-bit-specific. Of should it be somewhere outside of the mozconfig?

(In reply to Mike Kaganski from comment #110)

I have built TB now (since I started my work for LO, I always admire how fast TB is built) - but it turned out to be 32-bit. I see some mentions of mozconfigs for 64-bit, but they all lack anything 64-bit-specific. Of should it be somewhere outside of the mozconfig?

from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites#Common_problems_hints_and_restrictions

If you want to build 64-bit Firefox, add the two lines below to your mozconfig file:
ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32

Comment 112

2 months ago

(In reply to Kai Engert (:kaie:) from comment #108)

(Jörg's experimental patch attempted to remove the separate parameters from the call, and instead move them into the structure - which didn't work.)

Hmm, well, using the content from within the message does in fact work, see experiment 2 where I pass one file separately, yet you can still attach two. What doesn't work is dropping the parameters from the function, experiment 1.

Mike, I fixed https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build now. The MAPI stuff was on the Windows pre-requisites page, but it got removed and placed in the TB troubleshooting section :-(

As for 64bit: Yes, this is needed in your .mozconfig:

ac_add_options --enable-application=comm/mail
ac_add_options --enable-calendar
ac_add_options --enable-debug
ac_add_options --target=x86_64-pc-mingw32  <==
ac_add_options --host=x86_64-pc-mingw32    <==
ac_add_options --enable-warnings-as-errors
ac_add_options --enable-tests
(Assignee)

Comment 113

2 months ago

A couple more of off-topic things while I'm rebuilding:

  1. I believe ac_add_options --host=x86_64-pc-mingw32 should be redundant (a minor thing possibly not worth mentioning) - but overall, putting the 64-build hint to the TB build page would be fine (somewhere in Build configuration)
  2. On TB page, the requirement to run ./mach bootstrap is missing; and the logic of the text implies that ./mach build is run after cloning and creating mozconfig (as these steps are explicit on the page, it looks natural that the reference to Prerequisites ends with, say, installing VS for Windows) - but skipping the step will fail. Also, it's not evident which options to choose from the four options all mentioning FF; and if it's required to say "Yes" when it asks to create a .mozconfig (iirc) directory under user profile (it is).
(Assignee)

Comment 114

2 months ago

Well - while I'm testing, some thoughts.

The IP communications here are quite involved.

  1. An application finds the default mail provider in registry;
  2. It tries to load its DLL - it must be of correct bitness (32- or 64-);
  3. If it can, the DLL is run in the context of the caller, and talks to TB using COM; if it cannot, then it loads Windows MAPI dll, which possibly uses a shim to provide access to the default application with different architecture (as is the case when 64-bit application tries to send using 32-bit TB).

So all kinds of combinations are possible:

  • 64-bit app + 64-bit TB MAPI DLL -> 64-bit TB;
  • 32-bit app + 32-bit TB MAPI DLL -> 32-bit TB;
  • 64-bit TB app + 64-bit Windows MAPI dll -> shim -> (? 32-bit TB MAPI DLL ?) -> 32-bit TB;
  • 32-bit TB app + 32-bit Windows MAPI dll -> shim -> (? 64-bit TB MAPI DLL ?) -> 64-bit TB.

Of course, the memory management details may differ in these cases.

(Assignee)

Comment 115

2 months ago

Oh, there's another thing in play here: whether the calling app uses MAPISendMail or MAPISendMailW. Because in case of Windows 8+ (where the function is implemented in Windows' MAPI32.DLL), the latter would check if the TB MAPI DLL supports the W variant, and if not, it will internally convert passed arguments to 8-bit (allocating for that); so here the memory management may be complicated even more. The details are available in Windows SDK's MapiUnicodeHelp.h, which allows using MAPISendMailHelper (with wide-string semantics) even for Windows 7 (where MAPISendMailW is absent), using MAPISendMailW if available, and doing all the conversions itself.

(Assignee)

Comment 116

2 months ago

I'm really sorry for spamming. What is a better channel for asking questions like this:

I have built TB 64-bit; I followed comment 69 to register it (since I have 32-bit TB installed, I had to change HKEY_CLASSES_ROOT\WOW6432Node\CLSID{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}); when I use an application (LO) to send mail, I get MAPI_E_FAILURE from InitMozillaReference (CoCreateInstance fails with "no such interface" result for {29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}).

I suspect it's because of mismatch between 64-bit TB I test and 32-bit registry entries. So possibly I need to copy everything from 32-bit registry to 64-bit; or use an installer (mentioned above). I tried to copy HKEY_CLASSES_ROOT\WOW6432Node\CLSID{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7} to HKEY_CLASSES_ROOT\CLSID{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}, but it didn't change things. I couldn't find a 64-bit TB installer on thunderbird.net, neither did I find it in the build tree (checked for msi; for exe; for large-sized files - to no avail).

So what is the intended procedure to register it (I'd prefer to have manual steps with specific registry keys to change, to be able to revert it later - but of course, anything working is fine)?

Comment 117

2 months ago

I love the information, keep it coming ;-)

You have two options: mach package and install that, see comment 48. The other option would be to install a TB x64 version.

60.4.0: http://ftp.mozilla.org/pub/thunderbird/releases/60.4.0/win64/
Daily: http://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central/thunderbird-66.0a1.en-US.win64.installer.exe

(Assignee)

Comment 118

2 months ago

A sidenote.

Since nsMapiHook::HandleAttachments depends hard on nsMapiFileDesc.lpszPathName being wchar_t*, LPTSTR in nsMapiFileDesc (mailnews/mapi/mapihook/build/msgMapi.idl) must be LPWSTR (to not depend on some UNICODE defines).

(Assignee)

Comment 119

2 months ago

Ok, this is my analysis.

mozMAPI32.dll's MAPISendMail may be called with lpMessage == nullptr. The memory which lpMessage points to is never modified inside the function; but nsIMapi::SendMail (CMapiImp::SendMail) is made sure to be called with all valid pointers; the pointers are:

  • aMessage
  • aRecips
  • aFiles

Here, aRecips and aFiles may point to aMessage->lpRecips and aMessage->lpFiles, or they may point to some zeroed memory blocks in case aMessage has those nullptrs. The latter may be because lpMessage had these nullptrs, or because lpMessage was nullptr itself, thus a zeroed memory block is used instead.

When passed to MAPI hook via COM, the data is marshaled using MapiProxy.dll (built from .c files auto-generated by MIDL compiler). The marshaling copies every block of data which (recursively for all pointers inside structs!), so when it arrives into CMapiImp::SendMail, the pairs [aMessage->lpRecips and aRecips], [aMessage->lpFiles and aFiles] are guaranteed to point to different addresses. Note that aMessage->lpRecips and aMessage->lpFiles may or may not be nullptr here!

Then aMessage->lpRecips and aMessage->lpFiles are unconditionally reset to aRecips and aFiles resp.; thus the previous addresses of memory they pointed to are lost (no idea if they are somehow tracked by COM), and aRecips and aFiles blocks are referenced twice by data managed by COM.

What it means in the end is that some memory may be destroyed twice, while some may be lost.

What I'd expect to be the correct approach here would be this:

  1. Change the CMapiImp::SendMail to follow MAPISendMail signature (as Jorg did in experiment 1);
  2. Just send lpMessage to CMapiImp::SendMail as it arrives to MAPISendMail (msgMapi.idl already has this param set to [unique], so the comments about the need to handle nullptrs specially are likely obsolete - see [1]).
    The CMapiImp::SendMail and following processing already is prepared for nullptrs in aMessage->lpRecips and aMessage->lpFiles. Just need a little tweak to handle aMessage itself.

In the meanwhile, it's unclear to me how does passing of nsMapiFileDesc happen, when lpszPathName and lpszFileName are actually char* (and so aFlags & MAPI_UNICODE is 0). The IDL used by TB knows that they are wchar_t*, so the marshaling should use wchar_t* zero-termination to copy strings; but if char* is passed, then it's not guaranteed to have the two trailing zeroes... but possibly that's another story, and maybe it somehow is handled on the caller side, if it uses the IDL "properly" (shim code is auto-created?).

It's incorrect to ignore the MAPI_UNICODE flag in Flags (as in the v4) - it's the caller's decision how to use this, as part of MS MAPI.

It should not be required to do the copy of the data in the MAPISendMail.

I'm preparing my attempt.

[1] https://blogs.msdn.microsoft.com/oldnewthing/20140919-00/?p=44023

Comment 120

2 months ago

(In reply to Mike Kaganski from comment #118)

Since nsMapiHook::HandleAttachments depends hard on nsMapiFileDesc.lpszPathName being wchar_t*, LPTSTR in nsMapiFileDesc (mailnews/mapi/mapihook/build/msgMapi.idl) must be LPWSTR (to not depend on some UNICODE defines).

I don't understand this comment. The patch changes lpszPathName to LPSTR and HandleAttachments() does pFile->InitWithNativePath(nsDependentCString(aFiles[i].lpszPathName)); - No wide string far and wide to be seen. So you're saying this is wrong?

(Assignee)

Comment 121

2 months ago

(In reply to Jorg K (GMT+1) from comment #120)

Well - actually I only started to look into the patch details after I had a chance to look around in the original code (so the comment 118 was made before I realized that change).

Now I'm confused about the possibility to use whcar_t* in MAPISendMail. Possibly I'm mistaken, and wchar_t* in only sent to MAPISendMailW... then the change is fine. Well - it's orthogonal to the memory handling / crashing here.

(Assignee)

Comment 122

2 months ago

Heh, [1], [2] and [3] suggests only one thing: MAPI_UNICODE is invalid flag to MAPISendMail, and it's possible to have UTF-8 there using CP_UTF8 in MapiMessage::ulReserved. Well - that UTF-8 support should be outside of the scope here.

[1] https://docs.microsoft.com/en-us/windows/desktop/api/mapi/nc-mapi-mapisendmail
[2] https://docs.microsoft.com/ru-ru/windows/desktop/api/mapi/nc-mapi-mapisendmailw
[3] https://docs.microsoft.com/ru-ru/windows/desktop/api/mapi/ns-mapi-mapimessage

(Assignee)

Comment 123

2 months ago
Posted patch cleanup.diff (obsolete) — Splinter Review

This works for me :-) - this includes changing LPTSTR to LPSTR, as in v4.

Posted patch potential minimal crash fix (obsolete) — Splinter Review

Mike, thanks a lot for your great help and discovering the double free is caused by manipulating the incoming aMessage parameter to CMapiImp::SendMail.

I think the cleanup and fixes you've implemented seem great, and if it works, we should absolutely take your patch. However, IIUC, this smaller patch is the minimal cure for the crash.

Comment 125

2 months ago

Comment on attachment 9037162 [details] [diff] [review]
cleanup.diff

I like the clean-up. I'll try it when I'm back at my desk.

Flags: needinfo?(VYV03354)
(Assignee)

Comment 126

2 months ago

Wrt the LPCTSTR stuff, as I see the discussion here started from comment 59:

That wasn't a mistake on Mozilla side. I know that because I tried to work with this code 10 months ago, when MS haven't yet cleaned up their MAPI documentation pages into the new help system, as it is now. Then, there was no mention of MapiMessageW, and both MAPISendMail and MAPISendMailW documentation referred to common MapiMessage which actually had those LPCTSTR. That was the reason I thought it's how it's designed by MS, and wasn't terribly amazed seeing handling of MAPI_UNICODE in TB code. So it was just a matter of literally following existing MS documentation at that time actually.

(Assignee)

Comment 127

2 months ago

Wrt comment 81 ("The MS COM type lib code generated by MIDL for the MS COM interfaces checks for these parameters to be non null, although null is a valid value for them here"):

the relevant pieces here are:

  1. Initial import of comm-central into Mercurial from CVS - bug 437643 (2008) [1] which already contained the comment about inability to pass nullptrs.
  2. Change by Hiroyuki Ikezoe - bug 594224 (2010) which had changed (fixed) that.

So now indeed there's no need to make sure to not pass nullptrs; just need to make sure nullptr is handled correctly.

Wrt Jorg's comments re:tabs and other formatting: of course, my patch has problems with that - sorry.

[1] https://hg.mozilla.org/comm-central/rev/e4f4569d451a
[2] https://hg.mozilla.org/comm-central/rev/b9feb6dbd2a9

Comment 128

2 months ago

Mike, once again, thanks to much for your help. I know you're busy with LibreOffice these days, so this is even more appreciated. Once we're finished here (I guess today), we'll look at your patch in bug 1048658 after it's refreshed and benefited from all the knowledge gained here. And then, as a last favour I'd like to ask you to finish off bug 594239, so we have a trilogy ;-) - Part of the clean-up there is already happening here. After that, we'd have MAPI in a pretty good state :-)

Oh, one thing about UTF-8: This works today if you set your system locale to UTF-8, see comment #70. You don't need the MAPISendMailW if you use that option on Windows 10. However, prior experiments showed that Windows prefers to call the W interface, so no doubt we should implement it, also for users not using the "beta UTF-8" option.

Never mind the white-space issues, I'll fix those before checking it in. (Still trying to get TB built after switching office/machine.)

(Assignee)

Comment 129

2 months ago

Please disregard comment 126: my memory apparently failed me - as seen in my bug 1048658 comment 1 (so yes, MS documentation always defined the strings as 8-bit).

(Assignee)

Comment 130

2 months ago

(In reply to Jorg K (GMT+1) from comment #128)

I'm a TB user; and thus it's the least I can do to give back a tiny bit how your great product helps me :-)

Wrt UTF-8 and comment 70 my comment 122 is a different story. Of course, if setting Windows ACP to UTF-8, then all ACP-related conversions would work automatically. The CP_UTF8 as the value of MapiMessage::ulReserved is another story, which started initially as an undocumented "feature" of Outlook that gave them opportunity to use Unicode in Simple MAPI; they had documented that since then. It allows an application to pass UTF-8 to Simple MAPI regardless of ACP, and without (then-absent) MapiMessageW. I don't know if supporting that is of significant priority; just mention here that not it's a part of the official API.

Updated

2 months ago
Duplicate of this bug: 683857

Updated

2 months ago
Assignee: kaie → mikekaganski

Comment 132

2 months ago

See Bug 1430766 - Remove unused code from msgMapiHook.cpp
Should that be included in the cleanup?

Comment 133

2 months ago

Thanks, we'll look into it, not in this bug here, that's too long already.

Comment 134

2 months ago

This is Mike's patch with the tabs replaced, white-space removed and a comment tweaked, and with a proper commit message.

Sadly ... is doesn't work for me. Sending via "Send to > Mail recipient" starts TB, but no compose window. Running it from LibreOffice starts TB and then gives an error.

I've gone back to Kai's (v4) and that works.

This patch, just as my experiment 1 modifies CMapiImp::SendMail() and in my experiments that broke things when before Kai's (v3) and (v4) worked.

Since it's working for Mike, there must be something very subtle going on. Maybe I should build the TB 66 installer and install TB properly?

Attachment #9036387 - Attachment is obsolete: true
Attachment #9036440 - Attachment is obsolete: true
Attachment #9037162 - Attachment is obsolete: true
Attachment #9037169 - Attachment is obsolete: true
Attachment #9036387 - Flags: feedback?(mikekaganski)
(Assignee)

Comment 135

2 months ago

It needs to also tweak the registry path to MapiProxy.dll, since the marshalling would not work properly if your setup uses proxy from a different build (can't check the correct path ATM).

Comment 136

2 months ago

Sigh, this stuff is so brittle :-(

OK, I built the installer, and sending from LibreOffice works now. However, "Send to > Mail recipient" (tired of typing that, it'll be St>Mr in the future) doesn't. I've checked the two registry entries I mentioned before:

[HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID{29F458BE-8866-11D5-A3DD-00B0D0F3BAA7}\LocalServer32]
@=""C:\Program Files\Thunderbird Daily 66-self\thunderbird.exe" /MAPIStartup"

[HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla Thunderbird]
@="Mozilla Thunderbird"
"DLLPath"="C:\Program Files\Thunderbird Daily 66-self\mozMapi32_InUse.dll"

What's wrong still?

Comment 137

2 months ago

Comment on attachment 9037227 [details] [diff] [review]
393302-fix-crash-via-cleanup.patch

Sigh, it worked after logout/logon. No wonder my experiments never worked :-( - Maybe Windows as caching the DLL? I don't get it.

Let's go with this nice clean-up and move onto the follow-up bugs - yay \o/

Attachment #9037227 - Flags: review+

Updated

2 months ago
Attachment #9036603 - Attachment is obsolete: true
Attachment #9036603 - Flags: feedback?(mikekaganski)

Comment 138

2 months ago

Comment on attachment 9037227 [details] [diff] [review]
393302-fix-crash-via-cleanup.patch

Fast track this to TB 65 beta 3.

Attachment #9037227 - Flags: approval-comm-beta+

Comment 139

2 months ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/335361a67f2d
Correct memory handling in MAPISendMail() and CMapiImp::SendMail() to fix "Send to > Mail Recipient" crash. r=jorgk

Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED

Comment 140

2 months ago

Let's move onto bug 1048658, bug 594239 and potentially bug 1430766.

Comment 141

2 months ago
status-thunderbird65: --- → fixed
status-thunderbird66: --- → fixed
status-thunderbird_esr60: --- → affected

Comment 143

2 months ago

Fix commit message.

Attachment #9037303 - Attachment is obsolete: true
(Assignee)

Comment 144

2 months ago

By the way: the special treatment in the MAPISendMail:

// we are seeing a problem when using Word, although we return success from the MAPI support
// MS COM interface in mozilla, we are getting this error here. This is a temporary hack !!
if (hr == 0x800703e6)
hr = SUCCESS_SUCCESS;

is for 0x800703e6 which is wrapped Win32 error 998: "ERROR_NOACCESS Invalid access to memory location." [1]
Possibly it was the problem we fixed here.

[1] https://docs.microsoft.com/en-us/windows/desktop/Debug/system-error-codes--500-999-

(Assignee)

Comment 145

2 months ago

Testing with Word 2016 on my system, I see that it indeed does call MAPISendMail (even when MAPISendMailW is available!) with MapiMessage::ulReserved set to 65501 (CP_UTF8). Sigh.

Updated

2 months ago
Duplicate of this bug: 537413

Updated

2 months ago
Duplicate of this bug: 526540

Comment 148

2 months ago

(In reply to Jorg K (GMT+1) from comment #140)

Let's move onto bug 1048658, bug 594239 and potentially bug 1430766.

see also bug 547027 - Create a framework for MAPI tests
and bug 594243 - nsMapiHook::PopulateCompFieldsForSendDocs should create unique temporary file for attachments.

there are already patches for both bugs.

Comment 149

2 months ago

(In reply to Mike Kaganski from comment #145)

Testing with Word 2016 on my system, I see that it indeed does call MAPISendMail (even when MAPISendMailW is available!) with MapiMessage::ulReserved set to 65501 (CP_UTF8). Sigh.

I've seen MAPISendMailW() being called by St>Mr.

(Assignee)

Comment 150

2 months ago

(In reply to Jorg K (GMT+1) from comment #149)

I've seen MAPISendMailW() being called by St>Mr.

Just to make sure: are you talking about MS Word in-built function, or about Windows Explorer context menu function (the latter indeed uses MAPISendMailW in my testing)? If the former, then possibly that's a Word version-dependent difference? (and possibly better discuss this at bug 1521007)

Comment 151

2 months ago

As I said: St>Mr = "Send to > Mail recipient" ;-) - You made me type it again.

(Assignee)

Comment 152

2 months ago

I'm sorry, and I got the meaning of the abbreviation; just I'm not sure if you refer to Explorer (which indeed has this), or Word (which could, possibly, has the same path in its menu in some version - I have only one at hand, and there are many Word versions released out there).

Comment 153

2 months ago

This fix will be available for TB60ESR (if i got it right).

Does anyone knows in which version it will be on?

Comment 154

2 months ago

Likely in TB 60.6, 2019-03-19.

Comment 155

2 months ago

(In reply to Jorg K (GMT+1) from comment #154)

Likely in TB 60.6, 2019-03-19.

OK, Thank you :)

Comment 156

2 months ago

(In reply to Mike Kaganski from comment #145)

Testing with Word 2016 on my system, I see that it indeed does call MAPISendMail (even when MAPISendMailW is available!) with MapiMessage::ulReserved set to 65501 (CP_UTF8). Sigh.

Umm, should we cater for this in a new bug? We always use NS_CopyNativeToUnicode() which will mess up if the system charset isn't UTF-8, but the data is. Do you get mojibake? If so, let's file a new bug, easy to fix checking that flag and using UTF-8 to UTF-16 instead. You're the person to fix it since I don't have Word 2016 ;-)

Flags: needinfo?(mikekaganski)
(Assignee)

Comment 157

2 months ago

(In reply to Jorg K (GMT+1) from comment #156)

Umm, should we cater for this in a new bug?

Heh, see last words of comment 150 ;-)

Flags: needinfo?(mikekaganski)

Comment 158

2 months ago
Comment on attachment 9037311 [details] [diff] [review]
393302-fix-crash-via-cleanup.patch - ESR60 version

Most likely for TB 60.6.
Attachment #9037311 - Flags: approval-comm-esr60+

Comment 159

2 months ago

I've decided to uplift some white-space changes, to this needed adjustment.

Attachment #9037311 - Attachment is obsolete: true
Attachment #9038021 - Flags: approval-comm-esr60+

Comment 160

2 months ago

The fix is also included in TB 65.0b3 x64?

Comment 161

2 months ago

Yes, and the follow-ups, bug 1048658 and bug 1521007 for full unicode support, will be in TB 65 beta 4 x64.

Comment 162

2 months ago

I tested TB 65 beta 3 x64. TB don't crash any more, but compose window won't open.

Comment 163

2 months ago

Please try a logon/logoff of better, restart of your machine. TB 65 beta 3 was installed with the official installer and nothing got installed afterwards? If not: Download the beta, install it, at least logon/off.

Comment 164

2 months ago

Still REPRODUCIBLE with unzipped installer of unofficial (by wg9s) en-US SeaMonkey 2.53 (NT 6.1; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0 Build 20190122130014 (Default Classic Theme, German Language Pack active) on German WIN7 64bit

status-seamonkey2.53: --- → affected

Rainer, if you simply unzipped, but didn't install, it cannot work. The updated DLL must be registered in the Windows system registry, please try to run the installer.

Comment 166

2 months ago

(In reply to Kai Engert (:kaie:) from comment #165)

Rainer, if you simply unzipped,

I used the "Make SM default Email application", and WIN7 went to admin mode. I think, that should have been sufficient to make SM "default mail application", but of course, I additionally I can run the installer. We will see.

Comment 167

2 months ago

Only the installer sets the registry entries that point to the MAPI DLL. This stuff is quite brittle, but if you get it installed properly, it works.

Rainer,
as Jorg and Kai told just you running the unzipped intaller won't work. The MAPI dll needs to be registered and this is only done by the installer. I tested with both libreoffice and "send to email" and 2.53 works fine here now.

(Assignee)

Comment 169

2 months ago

(In reply to Rainer Bielefeld from comment #166)

Hi Rainer,
Nice to see you here! o/ Just wanted to thank you for all the tremendous work you've done for LO!

Comment 170

2 months ago

(In reply to Frank-Rainer Grahl (:frg) from comment #168)

MAPI dll needs to be registered and this is only done by the installer.

Yup, so it is. I now use an installed Version of above mentioned Build 20190122130014, and now 'Sendfile as emailfrom WIN files Explorer' and also 'Send document by email'from LibO work fine.

It's a little strange, currently the buttons 'Make SeaMonkey default application for [Mail] and [News] both are active, so SeaMonkey seems to think that that currently it might NOT be the default application?
But if it's a problem that should be discussed for SM, not here in the Bug.
I think this one is FIXED for SM, and so I adapt the status flag (also considering frg's result).

status-seamonkey2.53: affected → fixed

I think this one is FIXED for SM, and so I adapt the status flag (also considering frg's result).

Keep in mind that these are official flags and mine and Bills version is anything but official. So unless it is checked into an, as of now not existing, SeaMonkey comm-release branch it is still affected.

status-seamonkey2.49esr: --- → affected
status-seamonkey2.53: fixed → ---

Comment 172

2 months ago

Keep in mind that these are official flags

you are right.

Updated

2 months ago
Duplicate of this bug: 618678

Comment 174

2 months ago

(In reply to Frank-Rainer Grahl (:frg) from comment #63)

Mail window with attachment is created in the background. But the z-order problem is either something SeaMonkey specific or present since ages too.

see Bug 758646 - (MAPI) TB Composition window opens in background when started from Windows Explorer > RightClick > Send to > Email recipient

Bug 758646 should be changed from MailNews Core to SeaMonkey if that does not happen in TB anymore.

Updated

2 months ago
Duplicate of this bug: 1524339

Updated

2 months ago
Duplicate of this bug: 1526055

Comment 177

a month ago
status-thunderbird_esr60: affected → fixed
tracking-thunderbird_esr60: --- → 65+
You need to log in before you can comment on or make changes to this bug.