1531869 - Automatic update does not replace MAPI DLLs

Status: RESOLVED FIXED

(Thunderbird :: Installer, defect)

Description

[Jorg K (CEST = GMT+2)]

We had quite some drama when shipping changes to MAPI, see bug 1530820 with its bazillion duplicates.

Turn out that the automated update does not fully replace the MAPI DLLs.

Before the update: 60.5.1 DLLs:
C:\Program Files\Mozilla Thunderbird 60\MapiProxy_InUse.dll - 22 KB - 2019/02/13
C:\Program Files\Mozilla Thunderbird 60\mozMapi32_InUse.dll - 97 KB - 2019/02/13

Identical files MapiProxy.dll and mozMapi32.dll

After update to 60.5.2, these are still there:
C:\Program Files\Mozilla Thunderbird 60\MapiProxy_InUse.dll - 22 KB - 2019/02/13
C:\Program Files\Mozilla Thunderbird 60\mozMapi32_InUse.dll - 97 KB - 2019/02/13

And also:
C:\Program Files\Mozilla Thunderbird 60\mozMapi32.dll - 96 KB - 2019/03/01
C:\Program Files\Mozilla Thunderbird 60\MapiProxy.dll - 22 KB - 2019/03/01

2019/03/01 is the date of the auto-update.

As we can see, the mozMapi32.dll has changed, but the "InUse" version wasn't updated :-(

Nice mess. Richard, can you please take a look and ask someone in M-C?

Comment 1

[Richard Marti (:Paenglab)]

I see the same, when I update TB through the about dialog. When I use a installer then the two *_InUse.dll files are exchanged correctly.

Robert, we don't really know the installer code. Please could you look what is wrong with our installer code that it doesn't update the *_InUse.dll when run through the update?

Comment 2

[Richard Marti (:Paenglab)]

Robert, we have here a call for the MAPI DLL update: https://searchfox.org/comm-central/source/mail/installer/windows/nsis/installer.nsi#256

is this call also invoked when we upgrade from TB?

Comment 3

[Jorg K (CEST = GMT+2)]

Interesting. This code:
https://searchfox.org/comm-central/rev/1338c9e4036bfac79e922a0e9a6475b1ac24ec8e/mail/installer/windows/nsis/shared.nsh#1061
tries to copy like so:

CopyFiles /SILENT "$INSTDIR\MapiProxy.dll" "$INSTDIR\MapiProxy_InUse.dll"
CopyFiles /SILENT "$INSTDIR\mozMapi32.dll" "$INSTDIR\mozMapi32_InUse.dll"

What we see is that the non-InUse DLLs get upgraded, but the copy isn't made. So either that macro doesn't run or fails.

Comment 4

[Richard Leger]

Attachment: MapiDiff.png

Hi,

I don't know if that can help, but there seems to be two different set of code to proceed with the mapi dll replacement in https://searchfox.org/comm-central/source/mail/installer/windows/nsis/installer.nsi

At line 256 (under Section "-InstallStartCleanup") which call macro ${UpgradeMapiDLLs} which lines of code seems defined here https://searchfox.org/comm-central/rev/1338c9e4036bfac79e922a0e9a6475b1ac24ec8e/mail/installer/windows/nsis/shared.nsh#1061

At line 285 (under Section "-Application" APP_IDX) which does not use the macro and define slightly different lines of code and paths...

See different attached... especially path seems to differ...

Could that be cause of the issue?

Also noted the macro does not seems to log anything so you may not know if it is triggered or not at the moment by looking at log files...

I don't know if that can help sort the ongoing issue somehow but I thought to mention it...

Regards,

Comment 5

[Richard Leger]

Also, you may also want to consider to trigger an error if the CopyFiles fails or at least log the error in case of failure to notice it perhaps in the future...

Comment 6

[Jorg K (CEST = GMT+2)]

Thanks, Richard. Please always paste permalinks.

So:
https://searchfox.org/comm-central/rev/1338c9e4036bfac79e922a0e9a6475b1ac24ec8e/mail/installer/windows/nsis/installer.nsi#256
calling this:
https://searchfox.org/comm-central/rev/1338c9e4036bfac79e922a0e9a6475b1ac24ec8e/mail/installer/windows/nsis/shared.nsh#1061

and some similar code here:
https://searchfox.org/comm-central/rev/1338c9e4036bfac79e922a0e9a6475b1ac24ec8e/mail/installer/windows/nsis/installer.nsi#283

I have no idea whether that runs during automated update and what would happen on error if a DLL was locked.

Comment 7

[Mike Kaganski]

Hmm... The update process looks indeed odd.

It tries to delete an _InUse dll; and failing that, it tries to rename the dll, and deletes that renamed dll "after reboot" (i.e., it likely uses MoveFileEx [1] with MOVEFILE_DELAY_UNTIL_REBOOT). The latter writes to HKLM\System\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations to delete it immediately after AUTOCHK (as described in [2], [3], [4]).

The problem here seems to be that the procedure relies upon the ability to rename a used dll. It might be wrong in some versions of Windows... so the correct procedure would be to write the new _InUse dll with a temporary name, and then sequentially add two MoveFile calls - one to remove the old _InUse dll (with original name), and then to rename the new one with temporary name into the original name. The entries added to HKLM\System\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations are executed in the order in which they are added there; so this would make the desired effect.

I don't know what is the NSIS API to use for delayed-until-reboot renames, though.

[1] https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-movefileexw
[2] https://support.microsoft.com/en-us/help/181345/how-to-replace-in-use-files-at-windows-restart
[3] https://blogs.technet.microsoft.com/brad_rutkowski/2007/06/27/are-there-pending-operations-waiting-for-a-reboot/
[4] https://docs.microsoft.com/en-us/sysinternals/downloads/movefile

Comment 8

[Mike Kaganski]

But then - some users indicate that simply reinstalling of the same version over existing TB fixes the problem. Does that indicate that the function actually works - but when auto-updating, does not get executed? Or those users happen to update when they haven't yet used MAPI, thus have MAPI DLLs not locked, so that is just a coincidence?

Comment 9

[Robert Strong (they/them - no direct email)]

This was caused by bug 1336804. It removed the call to the UpgradeMapi macro in the PostUpdate macro.

-- a/mail/installer/windows/nsis/shared.nsh
+++ b/mail/installer/windows/nsis/shared.nsh
@@ -7,41 +7,30 @@
 
   ; Remove registry entries for non-existent apps and for apps that point to our
   ; install location in the Software\Mozilla key and uninstall registry entries
   ; that point to our install location for both HKCU and HKLM.
   SetShellVarContext current  ; Set SHCTX to the current user (e.g. HKCU)
   ${RegCleanMain} "Software\Mozilla"
   ${RegCleanUninstall}
   ${UpdateProtocolHandlers}
-  ; Win7 taskbar and start menu link maintenance
-  Call FixShortcutAppModelIDs
 
   ; setup the application model id registration value
   ${InitHashAppModelId} "$INSTDIR" "Software\Mozilla\${AppName}\TaskBarIDs"
 
-  ; Upgrade the copies of the MAPI DLL's
-  ${UpgradeMapiDLLs}
-
-  ; Delete two files installed by Kaspersky Anti-Spam extension that are only
-  ; compatible with Thunderbird 2 (bug 533692).
-  ${If} ${FileExists} "$INSTDIR\components\klthbplg.dll"
-    Delete /REBOOTOK "$INSTDIR\components\klthbplg.dll"
-  ${EndIf}
-  ${If} ${FileExists} "$INSTDIR\components\IKLAntiSpam.xpt"
-    Delete /REBOOTOK "$INSTDIR\components\IKLAntiSpam.xpt"
-  ${EndIf}
+  ; Win7 taskbar and start menu link maintenance
+  Call FixShortcutAppModelIDs

Comment 10

[Jorg K (CEST = GMT+2)]

:-(

Thanks for looking, Robert. So we'll just reinstate this and we'll be in business?

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1531869-update-mapi.patch

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 9048109 [details] [diff] [review]
1531869-update-mapi.patch

># HG changeset patch
># User Jorg K <[e-mail]>
># Parent  6fb9189f3a628e1bd1af3a91c004ce25a99b102a
>Bug 1531869 - Fix auto-update of MAPI DLLs be reinstating accidentally removed code (bug 1336804, rev 6c8ca70fc623). r=rstrong
>
>diff --git a/mail/installer/windows/nsis/shared.nsh b/mail/installer/windows/nsis/shared.nsh
>--- a/mail/installer/windows/nsis/shared.nsh
>+++ b/mail/installer/windows/nsis/shared.nsh
>@@ -29,16 +29,19 @@
>     StrCpy $TmpVal "HKLM" ; used primarily for logging
>     ${RegCleanMain} "Software\Mozilla"
>     ${RegCleanUninstall}
>     ${UpdateProtocolHandlers}
> 
>     ; Win7 taskbar and start menu link maintenance
>     Call FixShortcutAppModelIDs
> 
>+    ; Upgrade the copies of the MAPI DLL's
>+    ${UpgradeMapiDLLs}
This isn't quite right. This should be added back just below the first 'Call FixShortcutAppModelIDs' above this in PostUpdate... the one after ${InitHashAppModelId}. Doing this here will only upgrade the mapi dll's for users that don't have write access to HKLM.

Comment 13

[Robert Strong (they/them - no direct email)]

Actually, doing this here will only upgrade the mapi dll's for users that do have write access to HKLM. The change needed is still the same though.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1531869-update-mapi.patch (v2)

Thanks Robert. Yes, I inserted it after the wrong FixShortcutAppModelIDs, sorry.

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 9048118 [details] [diff] [review]
1531869-update-mapi.patch (v2)

This should do it

Comment 16

[Jorg K (CEST = GMT+2)]

Comment on attachment 9048118 [details] [diff] [review]
1531869-update-mapi.patch (v2)

Thanks again, Robert.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4509e764d895
Fix auto-update of MAPI DLLs by reinstating accidentally removed code (bug 1336804, rev 6c8ca70fc623). r=rstrong DONTBUILD

Comment 18

[Jorg K (CEST = GMT+2)]

TB 60.5.3:
https://hg.mozilla.org/releases/comm-esr60/rev/8c8f49c6968e30b5ebb62d5a3d3abb2dd581c2e7

Comment 19

[Jorg K (CEST = GMT+2)]

TB 66 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/b687193bb451cf1109421df9087f90465dda2ed5