Access violation when checking for updates #201

peters · 2013-10-13T10:24:38Z

Version: 0.7.2

Please note that the update directory is empty, and i just wanted to test what
Shimmer does in that case. Appearently it results in a epic crash.

Also access violation is also observed in these scenarios:

You specify an invalid url (borked URI)
The folder containing updates does not exist

So.... This is not quite ready for prime time i guess 🚒

using (var updateManager = new UpdateManager(OdinKapitalSettings.UpdateUrl, "odinkapital", FrameworkVersion.Net45))
            {
                btnSettingsCheckForUpdates.Enabled = false;
                var updateInfo = await updateManager.CheckForUpdate();
                if (updateInfo == null || !updateInfo.ReleasesToApply.Any())
                {
                    MessageBox.Show(this, "No updates available.");
                    btnSettingsCheckForUpdates.Enabled = true;
                }
                else
                {
                    if (DialogResult.OK != 
                        MessageBox.Show(this, "A new version is available, do you want to continue?", "New update available", MessageBoxButtons.OKCancel))
                    {
                        return;
                    }
                    var releases = updateInfo.ReleasesToApply;
                    await updateManager.DownloadReleases(releases);
                    await updateManager.ApplyReleases(updateInfo);
                    Application.Restart();
                }
            }

Log

[WARN][2013-10-13T12:21:37] UpdateManager: Failed to load local release list: System.IO.FileNotFoundException: Could not find file 'C:\Users\peters\AppData\Local\odinkapital\packages\RELEASES'.
File name: 'C:\Users\peters\AppData\Local\odinkapital\packages\RELEASES'
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, Boolean useAsync)
   at System.IO.FileInfo.OpenRead()
   at System.IO.Abstractions.FileInfoWrapper.OpenRead()
   at Shimmer.Client.UpdateManager.checkForUpdate(Boolean ignoreDeltaUpdates, IObserver`1 progress)
[INFO][2013-10-13T12:21:37] UpdateManager: Downloading RELEASES file from http://www.odinkapital.no/shimmer/updates

Partial stack trace

A first chance exception of type 'System.IO.FileNotFoundException' occurred in mscorlib.dll
A first chance exception of type 'System.Exception' occurred in Shimmer.Core.dll
A first chance exception of type 'System.Exception' occurred in mscorlib.dll
The thread 'Event Loop 1' (0xb94) has exited with code 0 (0x0).
A first chance exception of type 'System.Exception' occurred in mscorlib.dll
A first chance exception of type 'System.Reflection.TargetInvocationException' occurred in mscorlib.dll
A first chance exception of type 'System.IO.FileNotFoundException' occurred in mscorlib.dll
A first chance exception of type 'System.IO.FileNotFoundException' occurred in mscorlib.dll
'odinkapital.vshost.exe' (Managed (v4.0.30319)): Loaded 'Microsoft.GeneratedCode'
The program '[7008] odinkapital.vshost.exe: Program Trace' has exited with code 0 (0x0).
The program '[7008] odinkapital.vshost.exe: Managed (v4.0.30319)' has exited with code -1073741819 (0xc0000005) 'Access violation'.

The text was updated successfully, but these errors were encountered:

shiftkey · 2013-10-13T10:32:10Z

You specify an invalid url (borked URI)

The folder containing updates does not exist

Catching those errors and surfacing them as ShimmerConfigurationExceptions (and updating the sample to demonstrate those issues) is something I had started implementing (but put aside because of other priorities).

Now that someone else has hit the same issue, I feel much more confident about adding those in, for real.

shiftkey · 2013-10-13T10:53:43Z

Oops, I'm not thinking of something I planned to do, but something I'd already shipped:

d450bac

Ok, so coming back to your code above - there's two things we could do in this case:

Throw an exception

Pros:

blindlingly obvious when things aren't right
exceptions can point users to documentation explaining configuration/setup

Cons:

everyone has to check for this (which kinda sucks)

Don't throw an exception

Pros:

simpler code for consumers

Cons:

"why doesn't my update work?" issues being raised (which could be offset by current logging messages)

I'm still leaning towards the first scenario (why the exception messages aren't being raised correctly is something I'll look into) - how would you expect this "configuration is wrong" message to appear to your application?

peters · 2013-10-13T11:56:19Z

Throw an exception
blindlingly obvious when things aren't right

Exception based flow control is tha shit!

everyone has to check for this (which kinda sucks)

Let the exceptions buble -> e.g AggregateException

how would you expect this "configuration is wrong" message to appear to your application?

Hmm, that's a though one. Maybe something like this:

InvalidUpdateUrlException (Maybe use default framework exception here?)
UpdateFolderNotFoundException (could not establish connection, unable to list folder content)
CheckForReleaseException (some internal state in shimmer prevents checking for updates)
DownloadReleaseException (unable to download update)
ApplyReleaseException (unable to apply release(s))

These exceptions should inherit from UpdateAggregateException so you only need to declare a catch clause
around the super.

Find new names please, these are terrible 👯

shiftkey · 2013-10-13T11:56:48Z

So I added a test like this:

[Fact]
public void WhenFolderDoesNotExistThrowHelpfulErrorAsync()
{
    string tempDir;
    using (Utility.WithTempDirectory(out tempDir))
    {
        var directory = Path.Combine(tempDir, "missing-folder");
        var fixture = new UpdateManager(directory, "MyAppName", FrameworkVersion.Net40);

        using (fixture)
        {
            AssertEx.TaskThrows<ShimmerConfigurationException>(
                async () => await fixture.CheckForUpdate());
        }
    }
}

Gives me this lovely error:

I suspect I need to revisit the behaviour of the custom awaiter we're using here as there's too much wrapping of crap (basically).

Does that line up with what you're seeing in your testing?

peters · 2013-10-13T12:00:47Z

Does that line up with what you're seeing in your testing?

Yes, sir!

peters · 2013-10-13T12:07:24Z

Catching those errors and surfacing them as ShimmerConfigurationExceptions (and updating the sample to demonstrate those issues) is something I had started implementing (but put aside because of other priorities).

Cool, solves #201 (comment)

shiftkey · 2013-10-13T12:12:03Z

@peters ok great, that gives me more to go with

In terms of raising exceptions, these are my current thoughts on it:

Things which need to be addressed (either by the user or by the developers who wrote the application) should be raised and handled accordingly. If an update cannot be done (for whatever reason) it should not impact the behaviour of the application. Don't nag the user and don't crash the app - the user doesn't care, unless they're updating the application by hand.

InvalidUpdateUrlException (Maybe use default framework exception here?)
UpdateFolderNotFoundException (could not establish connection, unable to list folder content)

I'd group these together with a helpful error message for each - they both occur within CheckForUpdates so having multiple catch-es feels redundant.

CheckForReleaseException (some internal state in shimmer prevents checking for updates)

We're now ensuring we return an UpdateInfo result always - I'm not sure what might come up here aside from the preconditions discussed above. Timeout are the big headache, I think we should just log those rather than interrupt the user. Same thing with connectivity (or lack of).

DownloadReleaseException (unable to download update)

Two things can come up here:

404 (that's bad, and quite possibly a permanent issue)
Timeout (less bad, more likely to be transient)

I think the first one is worth interrupting the user for. The second one, not really.

ApplyReleaseException (unable to apply release(s))

This is where we get into the hairy stuff, things like:

the checksum validation fails (can't update! :( )
file corruption (maybe!)

These should interrupt, but how much can be resolved at this point is hard to say.

These exceptions should inherit from UpdateAggregateException so you only need to declare a catch clause
around the super.

If you want to swallow the exceptions, just listen for Exception and react accordingly. If you want to catch specific exceptions, declare them before.

I've got background updating to implement for the desktop demo - I'll use async/await there as a way of fleshing out these bugs and scenarios.

peters · 2013-10-13T12:21:41Z

Two things can come up here:
404 (that's bad, and quite possibly a permanent issue)
Timeout (less bad, more likely to be transient)

Yeah, this is pretty hairy stuff. I'm thinking maybe monotorrent could
be fit for Shimmer (as an alternative). Obviously not everybody has the resources to host artifacts in the cloud (torrent metadata), but i'm seriously thinking about doing just so because deployments in the future might be in "hostile" environments where you never, ever, never, ever are going to get 100% of the bits you need.

I've used monotorrent in a project before, it's simple to use.

Probably post 1.0 though 🐹

peters · 2013-10-13T12:22:03Z

I've got background updating to implement for the desktop demo - I'll use async/await there as a way of fleshing out these bugs and scenarios.

Nice!

shiftkey · 2013-10-13T21:13:28Z

Obviously not everybody has the resources to host artifacts in the cloud (torrent metadata), but i'm seriously thinking about doing just so because deployments in the future might be in "hostile" environments where you never, ever, never, ever are going to get 100% of the bits you need.

We've been looking to use BITS to support more robust downloading of updates, but this is definitely a post-1.0 feature...

peters · 2013-10-13T21:47:20Z

@shiftkey Hmm, i'm trying to exclude two packages from my nuspec dependency list because these are being
obsfucated by SmartAssembly, but using the following nuspec (the files are still being included):

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
  <metadata>
    ... bla bla bla ...
    <dependencies>
      <dependency id="AbbyyOnlineSdk" version="$version$" />
      <dependency id="AsposeCloud.SDK" version="$version$" />
      <dependency id="HigLabo" version="$version$" />
      <dependency id="Magick.NET-Q16-x86" version="$version$" />
      <dependency id="ModernUI" version="$version$" />
      <dependency id="NLog" version="$version$" />
      <dependency id="protobuf-net" version="$version$" />
      <dependency id="semver" version="$version$" />
      <dependency id="ServiceStack.Text" version="$version$" />
      <dependency id="Tesseract" version="$version$" />
      <dependency id="xps2img" />
     <!- two packages has been purposely left out here -->
    </dependencies>
  </metadata>
</package>

Any ideas on how i this can be achieved?

peters · 2013-10-13T21:50:13Z

Edit: The nupkg is generated correctly, but it's shimmer that references them (full pkg).

shiftkey · 2013-10-13T21:58:48Z

Inside the packages.config specify developmentDependency="true" for whichever packages should be excluded.

peters · 2013-10-13T22:00:53Z

👍 👍 👍 👍 👍

shiftkey · 2013-10-14T01:48:03Z

Right, so I'm now stuck between a rock and a hard place on this issue.

When you throw an exception after the fact (like we do when we await an AwaitableAsyncSubject), the stack trace is thrown away, like this:

Without a really nice message (or stacktrace) these issues were almost impossible to resolve without a debugger attached. I wanted something better, which meant wrapping the exception to preserve the stacktrace.

Time to spend some time digging into CLR bits

peters · 2013-10-26T09:35:35Z

@shiftkey In Shimmer 0.7.4 suddenly all project references (not nuget packages) are by default added to the nupkg and this was not the case before. Trying to override with <references /> but still has no effect. The reason this is important is that they need to be obsfucated. This could be solved with an AfterCompile target, but there's a bug in SmartAssembly so this does not work either. Ideas?

anaisbetts · 2013-10-26T18:11:18Z

Without a really nice message (or stacktrace) these issues were almost impossible to resolve without a debugger attached. I wanted something better, which meant wrapping the exception to preserve the stacktrace.

This is only fixed in .NET 4.5, if you're using 4.0 you're up a creek :(

peters · 2013-10-26T19:18:06Z

@paulcbetts I eventually moved the entire build process out of visual studio and ending up with
creating the nupkg myself. Here's the code snippet i used for creating a squirrel release:

function MyGet-Squirrel-New-Release {
    param(
        [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)]
        [string]$solutionFolder,
        [Parameter(Position = 1, Mandatory = $true, ValueFromPipeline = $true)]
        [string]$buildFolder
    )

    $packagesDir = Join-Path $solutionFolder "packages"

    $commandsPsm1 = MyGet-Grep -folder $packagesDir -recursive $true `
        -pattern "Shimmer.*commands.psm1$" | 
        Sort-Object FullName -Descending |
        Select-Object -first 1

    if(-not (Test-Path $commandsPsm1.FullName)) {
        MyGet-Die "Could not find any Squirrel nupkg's containing commands.psm1"
    }  

    MyGet-Write-Diagnostic "Squirrel: Creating new release"  

    Import-Module $commandsPsm1.FullName

    New-ReleaseForPackage -SolutionDir $solutionFolder -BuildDir $buildFolder

}

peters · 2013-11-20T10:44:24Z

In shimmer 0.7.4 automatic updates are still broken. I get the following error message when attempting
to update one of my applications:

11/20/2013 11:42:37 Thread: AutoKollekt - Downloading 1 updates. 
11/20/2013 11:42:44 Thread: AutoKollekt - Applying 1 updates. 
11/20/2013 11:42:44 Thread: AutoKollekt - Error when updating application EXCEPTION OCCURRED:System.InvalidOperationException: AwaitableAsyncSubject<T>.GetResult() is rethrowing an inner exception ---> System.InvalidOperationException: AwaitableAsyncSubject<T>.GetResult() is rethrowing an inner exception ---> System.IO.FileLoadException: Could not load file or assembly 'NuGet.Core, Version=2.7.40808.167, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
   at Shimmer.Client.UpdateManager.installPackageToAppDir(UpdateInfo updateInfo, ReleaseEntry release)
   at Shimmer.Client.UpdateManager.<>c__DisplayClass1d.<applyReleases>b__1a()
   at System.Reactive.Linq.QueryLanguage.<>c__DisplayClasscb`1.<>c__DisplayClasscd.<ToAsync>b__ca()
   --- End of inner exception stack trace ---
   at ReactiveUIMicro.AwaitableAsyncSubject`1.GetResult()
   at Shimmer.Client.UpdateManager.<applyReleases>d__1f.MoveNext()
   --- End of inner exception stack trace ---
   at ReactiveUIMicro.AwaitableAsyncSubject`1.GetResult()
   at odi.autokollekt.Config.AutoKollektApiHandler.<>c__DisplayClass2c.<<Maintenance>b__2a>d__2e.MoveNext()    at ReactiveUIMicro.AwaitableAsyncSubject`1.GetResult()
   at odi.autokollekt.Config.AutoKollektApiHandler.<>c__DisplayClass2c.<<Maintenance>b__2a>d__2e.MoveNext()

peters · 2013-11-20T10:45:27Z

Assembly manifest

<?xml version="1.0" encoding="utf-8"?>
<configuration>
    <startup> 
        <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5" />
    </startup>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Data.SQLite" publicKeyToken="db937bc2d44ff139" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.0.89.0" newVersion="1.0.89.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="Common.Logging" publicKeyToken="af08829b84f0328e" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-2.0.0.0" newVersion="2.0.0.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="System.IO.Abstractions" publicKeyToken="d480b5b72fb413da" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.4.0.68" newVersion="1.4.0.68" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="NuGet.Core" publicKeyToken="31bf3856ad364e35" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-2.7.40911.225" newVersion="2.7.40911.225" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>
</configuration>

Specify Shortcut Locations on Update.exe

peters closed this as completed Nov 20, 2013

peters reopened this Nov 20, 2013

akrisiun pushed a commit to akrisiun/Squirrel.Windows that referenced this issue Dec 2, 2015

Merge pull request Squirrel#201 from Squirrel/specify-shortcut-locations

6b75184

Specify Shortcut Locations on Update.exe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Access violation when checking for updates #201

Access violation when checking for updates #201

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 14, 2013

peters commented Oct 26, 2013

anaisbetts commented Oct 26, 2013

peters commented Oct 26, 2013

peters commented Nov 20, 2013

peters commented Nov 20, 2013

Access violation when checking for updates #201

Access violation when checking for updates #201

Comments

peters commented Oct 13, 2013

Log

Partial stack trace

shiftkey commented Oct 13, 2013

shiftkey commented Oct 13, 2013

Throw an exception

Don't throw an exception

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 13, 2013

peters commented Oct 13, 2013

shiftkey commented Oct 14, 2013

peters commented Oct 26, 2013

anaisbetts commented Oct 26, 2013

peters commented Oct 26, 2013

peters commented Nov 20, 2013

peters commented Nov 20, 2013