[wix-devs] OnExecutePackageComplete without OnExecutePackageBegin

Heath Stewart Heath.Stewart at microsoft.com
Thu Mar 17 10:10:51 PDT 2016

So how do people - Rob and Bob in particular - feel about an early return (i.e. "return S_OK") in this case? Since this may, in fact, set a precedent I'd like to know what's acceptable and I can code up a fix.


Heath Stewart
Visual Studio, Microsoft

-----Original Message-----
From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On Behalf Of Heath Stewart
Sent: Wednesday, March 16, 2016 1:45 PM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Subject: Re: [wix-devs] OnExecutePackageComplete without OnExecutePackageBegin

OnCache* is fine. I was using that as an example of what is, IMO, coded correctly. But when package execution begins it checks the cache result and bails before telling the BA package execution started (which is actually in each engine's execute functions), but the general LExit block calls OnExecutePackageComplete. That's all in the code.

>From Rob, I'll file a bug and get a PR out, but proposing a true early termination via return.

Here's the gist of the code:

Execute*Package(...) {
  if (pPackage->hrCacheResult) {
    ExitFunction1(hr = S_OK); // propose to just return S_OK;
  // other stuff, all which often uses Exit*() macros
  hr = ExecutePackageComplete(...); // calls ba->OnPackageExecuteComplete
  return hr;

It makes sense we bail from execute() if caching failed (no package on which to execute, in most cases anyway) but execute() was already scheduled. The runtime couldn't have known (practically) that caching would fail, so the extra check makes sense. But calling the ExitFunction macro ends up calling OnExecutePackageComplete, so we either return early or maybe skip to another label like LExit2 after LExit, in which case any future general cleanup could be done. I don't see any precedent so curious what others think.

Heath Stewart
Visual Studio, Microsoft

-----Original Message-----
From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On Behalf Of Sean Hall
Sent: Wednesday, March 16, 2016 10:10 AM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Subject: Re: [wix-devs] OnExecutePackageComplete without OnExecutePackageBegin

I think every Begin callback should always have a corresponding Complete callback, even if an error occurred.  A Complete callback should never be sent unless the corresponding Begin callback was sent, if Burn is doing this then I think that's a bug.

I'd have to look at code for the OnCacheBegin/OnCacheComplete question, if the whole cache phase is skipped I don't think they should be sent.

On Wed, Mar 16, 2016 at 12:03 PM, Heath Stewart <Heath.Stewart at microsoft.com
> wrote:

> We ran into a scenario where a caching error prevented 
> OnExecutePackageBegin from executing (basically, in apply.cpp, the 
> check for the caching HRESULT happens just before calling 
> OnExecutePackageBegin on the BA) but OnExecutePackageComplete was 
> called (it's the first line after the LExit label for each 
> package-specific execute function in apply.cpp. We can work around 
> this (just keep track of what packages were started and check that set 
> in the completion callback), but begs the question whether in burn we 
> should call a Complete callback without a corresponding Begin 
> callback. It's not common to see that sort of behavior (thinking about async APIs in various frameworks I've used).
> What is more consistent with typical async behavior - and allows a BA 
> to clean anything specific to the downloading/caching of a package 
> still - is that ApplyCache does call OnCacheBegin invariably (i.e. no 
> exits before said call) and OnCacheComplete in the LExit (again with 
> no early exits) so they are always a pair.
> So should this maybe changed for wix4?
> Heath Stewart
> Visual Studio, Microsoft
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fblogs.
> msdn.com%2fheaths&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c
> 84c7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sda
> ta=E%2bPWtOXEyuBU3%2bx7%2fZbPH50OTH1hwEOMy1WfFptWFek%3d
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant 
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.fi
> regiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c84c
> 7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=
WiX Toolset Developer Mailing List provided by FireGiant https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.firegiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c84c7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=AYTU4NnSFIqWFLIZvQqVPQPcma7lhgwNr6unMJBA0XI%3d
WiX Toolset Developer Mailing List provided by FireGiant https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.firegiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cf1621d6cadb1448b3ad308d34ddbcd82%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=znjVw8QcKUThgGCvUdXkqfvUAO4fL3RNk4iZ6DGzbV0%3d

More information about the wix-devs mailing list