Bug: Can't have more than 2 catches on try/catch blocks.

Kriskit's Avatar

Kriskit

18 Dec, 2017 10:39 AM

Hi,

I'm attempting to upgrade PostSharp from 4.0.36 to 5.0.41 (also upgrading to .NET 4.7 from 4.5.1) and I've found a bit of a bug.

After doing this, it started to fail builds with errors like the following:

Error    56  Unhandled exception (postsharp-net40-x86-srv.exe release | .NET Framework 4.7): System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
Parameter name: index
   at PostSharp.Sdk.CodeModel.MethodBodyDeclaration.GetLocalVariable(Int32 index, Boolean throwException)
   at PostSharp.Sdk.CodeModel.InstructionReader.GetLocalVariableSymbol(Int32 ordinal, Boolean createDefault)
   at ^NgJ.VYfo8e1N.^+fDKJEh9(StateMachineInfo _0, MethodDefDeclaration _1, InstructionBlock _2, InstructionSequence[] _3, Boolean _4, Boolean _5, LocalVariableSymbol& _6, LocalVariableSymbol& _7, InstructionBlock& _8, InstructionBlock& _9, InstructionSequence& _a, LocalVariableSymbol& _b, LocalVariableSymbol& _c, Int32& _d, LocalVariableSymbol& _e, LocalVariableSymbol& _f)
   at ^NgJ.VYfo8e1N.^+Quyshe4(StateMachineInfo _0, MethodDefDeclaration _1, InstructionBlock _2, LocalVariableSymbol _3, InstructionSequence[] _4, Boolean _5, Boolean _6, LocalVariableSymbol& _7, LocalVariableSymbol& _8, InstructionBlock& _9, InstructionBlock& _a, InstructionSequence& _b, LocalVariableSymbol& _c, LocalVariableSymbol& _d, Int32& _e, LocalVariableSymbol& _f, LocalVariableSymbol& _10)
   at ^PE9ycIe2cLwm.^HvVDHXOj(MetadataDeclaration _0, IDependencyTransformationInstance[] _1, PipelineTransversalState _2, MethodSemantics _3)
   at PostSharp.Sdk.AspectInfrastructure.AspectInfrastructureTask.^jwku0geZ(MetadataDeclaration _0, PipelineTransversalState _1, MethodSemantics _2, Boolean _3)
   at PostSharp.Sdk.AspectInfrastructure.AspectInfrastructureTask.^YVoDPIVQ(IMetadataDeclaration _0)
   at PostSharp.Sdk.AspectInfrastructure.StructuredDeclarationDictionary1.^d+wOzSPF(IMetadataDeclaration _0, Func2 _1)
   at PostSharp.Sdk.AspectInfrastructure.StructuredDeclarationDictionary1.^+g+TCqVg(TypeDefDeclaration _0, Func2 _1, Set1 _2)
   at PostSharp.Sdk.AspectInfrastructure.StructuredDeclarationDictionary1.^+g+TCqVg(TypeDefDeclaration _0, Func2 _1, Set1 _2)
   at PostSharp.Sdk.AspectInfrastructure.StructuredDeclarationDictionary1.^fJqG(Func2 _0)
   at PostSharp.Sdk.AspectInfrastructure.AspectInfrastructureTask.Execute()
   at PostSharp.Sdk.Extensibility.Project.ExecutePhase(String phase)
   at PostSharp.Sdk.Extensibility.Project.Execute()
   at PostSharp.Hosting.PostSharpObject.ExecuteProjects()
   at PostSharp.Hosting.PostSharpObject.InvokeProject(ProjectInvocation projectInvocation). Zen.Mediation.Authentication

After a lot of trial and error I determined that PostSharp does not like more than 2 catches with an exception variable in the last one:

            catch (FrozenAccountException)
            {
                return new PasswordLoginResult { Frozen = true };
            }
            catch (LockedAccountException)
            {
                return new PasswordLoginResult { Blocked = true };
            }
            catch (Exception ex)
            {
            }

If I remove the variable like so:

            catch (FrozenAccountException)
            {
                return new PasswordLoginResult { Frozen = true };
            }
            catch (LockedAccountException)
            {
                return new PasswordLoginResult { Blocked = true };
            }
            catch (Exception)
            {
            }

It now builds. However, if I wanted to use that last catch block, it will fail to build with the same error:

            catch (FrozenAccountException)
            {
                return new PasswordLoginResult { Frozen = true };
            }
            catch (LockedAccountException)
            {
                return new PasswordLoginResult { Blocked = true };
            }
            catch (Exception ex)
            {
                logger.Warn("LoginAsync", typeof(PasswordBasedAuthenticator), ex, "Login Failed");
            }

However, if I remove the second catch:

            catch (FrozenAccountException)
            {
                return new PasswordLoginResult { Frozen = true };
            }
            catch (Exception ex)
            {
                logger.Warn("LoginAsync", typeof(PasswordBasedAuthenticator), ex, "Login Failed");
            }
It builds successfully.

Fortunately, we don't need to actually do anything with the exception in the third catch in this case. I haven't got through all our projects yet so there may be places where we have this same situation but need to use the exception.

I hope this is useful in finding out what the problem is :)

  1. Support Staff 1 Posted by PostSharp Techn... on 18 Dec, 2017 11:38 AM

    PostSharp Technologies's Avatar

    Hello,

    I could not reproduce the error with a mockup of your types and try statement. Could you please describe what is specific to this method or better provide us with a reproduction of this issue?

    Thanks a lot!

    Best regards,
    Daniel

  2. 2 Posted by Kriskit on 18 Dec, 2017 11:48 AM

    Kriskit's Avatar

    Hi,

    I can't provide you with my code unfortunately and reproducing it in an isolated fashion might take more time than I can give right now.

    Not sure if this is useful or not, but I've just changed the following:

            public async Task<PasswordLoginResult> LoginAsync(Brand brand, ClientType clientType, string username, string password, string ipAddress, CultureInfo culture)
            {
                var credentials = credentialsFactory.CreatePasswordCredentials(username, password);
                try
                {
                    var result = await loginCommand.ExecuteAsync(brand, clientType, credentials, ipAddress, culture);
                    if (result.SessionToken != null)
                    {
                        var resolvedCulture = await sessionCultureResolver.ResolveAsync(culture, ipAddress);
                        var session = new Session
                                      {
                                          Username = result.Username,
                                          Token = result.SessionToken,
                                          Culture = resolvedCulture,
                                          CurrencyCode = CurrencyCode.GBP,
                                          PlayerCode = result.PlayerCode
                                      };
                        session = await sessionFactory.CreateSessionAsync(brand, session);
                        return new PasswordLoginResult { Session = session };
                    }
                }
                catch (FrozenAccountException)
                {
                    return new PasswordLoginResult { Frozen = true };
                }
                catch (LockedAccountException)
                {
                    return new PasswordLoginResult { Blocked = true };
                }
                catch (Exception ex)
                {
                }
                return new PasswordLoginResult();
            }
    

    To this, removing the async and returning Tasks:

            public Task<PasswordLoginResult> LoginAsync(Brand brand, ClientType clientType, string username, string password, string ipAddress, CultureInfo culture)
            {
                var credentials = credentialsFactory.CreatePasswordCredentials(username, password);
                try
                {
                    var result = loginCommand.ExecuteAsync(brand, clientType, credentials, ipAddress, culture).Result;
                    if (result.SessionToken != null)
                    {
                        var resolvedCulture = sessionCultureResolver.ResolveAsync(culture, ipAddress).Result;
                        var session = new Session
                                      {
                                          Username = result.Username,
                                          Token = result.SessionToken,
                                          Culture = resolvedCulture,
                                          CurrencyCode = CurrencyCode.GBP,
                                          PlayerCode = result.PlayerCode
                                      };
                        session = sessionFactory.CreateSessionAsync(brand, session).Result;
                        return Task.FromResult(new PasswordLoginResult { Session = session });
                    }
                }
                catch (FrozenAccountException)
                {
                    return Task.FromResult(new PasswordLoginResult { Frozen = true });
                }
                catch (LockedAccountException)
                {
                    return Task.FromResult(new PasswordLoginResult { Blocked = true });
                }
                catch (Exception ex)
                {
                }
                return Task.FromResult(new PasswordLoginResult());
            }
    

    And it built successfully.

  3. Support Staff 3 Posted by PostSharp Techn... on 18 Dec, 2017 02:18 PM

    PostSharp Technologies's Avatar

    Hello,

    I was not able to reproduce this again with the exactly same code and mock types (which are probably different) . Since it is async method, the bug may depend on quite small details. Could you please send us a reproduction?

    Also, which Visual Studio version are you using (including the minor version/revision)? Which .NET Framework version do you have installed on the machine (.NET 4.7 or .NET 4.7.1).

    Thanks a lot!

    Best regards,
    Daniel

  4. 4 Posted by Kriskit on 18 Dec, 2017 02:32 PM

    Kriskit's Avatar

    I'll see if I can get some time to make a reproduction, this has set me back a fair bit already having to exclude each code file from PostSharp to find out which file is causing the problem (it'd be nice if the error showed which class it's blowing up on), so trying to make up time.

    Using Visual Studio 2013 (version 12.0.40629.00 - Update 5) - Yeah we need to upgrade :)

    .NET version is 4.7.02053.

    Hope that helps.

  5. Support Staff 5 Posted by PostSharp Techn... on 18 Dec, 2017 03:19 PM

    PostSharp Technologies's Avatar

    Hello,

    I have managed to reproduce it under VS2013. So there is no need to send reproduction. IT demonstrates itself only under the old C# compiler and PostSharp 5.0.

    Thanks for reporting the issue, we'll be working on fix.

    (internal issue #15750)

    All the best,
    Daniel

  6. 6 Posted by Kriskit on 18 Dec, 2017 03:31 PM

    Kriskit's Avatar

    Ok thanks. We will hopefully get to upgrade to a newer version of Visual Studio, it's just finding the time.

    Just so you are aware, I have had this same issue with a different bit of code:

            private async Task<ICommunicationAttemptOutcome<T>> SendAsync<T>(
                IRequestConfiguration configuration, 
                HttpRequestMessage message)
                where T : IResponseMessage, new()
            {
                if (configuration.MetadataElements != null)
                    foreach (var metadataElement in configuration.MetadataElements)
                        message.Headers.Add(metadataElement.Name, metadataElement.Value);

            HttpResponseMessage response = null;
            try
            {
                try
                {
                    using (var wrapper = httpClientWrapperFactory.Create(configuration))
                        response = await wrapper.SendAsync(message);
                }
                catch (Exception e)
                {
                    return configuration.FailedCommunicationAttemptMapper.MapException&lt;T&gt;(e);
                }
    
                var contentString = await response.Content.ReadAsStringAsync();
    
                if (configuration.ResponseSuccessDeterminer.IsSuccessful&lt;T&gt;(response.IsSuccessStatusCode, contentString))
                {
                    if (configuration.SuccessfulResponseProcessor != null)
                        configuration.SuccessfulResponseProcessor.Process&lt;T&gt;(contentString);
    
                    if (response.StatusCode == HttpStatusCode.NoContent)
                        return new CommunicationAttemptOutcome&lt;T&gt;(new T());
    
                    T responseMessage;
                    if (configuration.DataSerialiser.TryDeserialise(contentString, out responseMessage))
                        return new CommunicationAttemptOutcome&lt;T&gt;(responseMessage);
                }
    
                return configuration.FailedCommunicationAttemptMapper.MapUnsuccessfulResponse&lt;T&gt;(contentString);
            }
            finally
            {
                if (response != null)
                {
                    var logger = serviceLocator.Resolve&lt;ILogger&gt;();
                    try
                    {
                        logger.Trace(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), &quot;Disposing of HttpResponseMessage&quot;);
                        response.Dispose();
                        logger.Info(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), &quot;Disposed of HttpResponseMessage&quot;);
                    }
                    catch (Exception e)
                    {
                        logger.Warn(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), e, &quot;Unable to dispose of HttpResponseMessage&quot;);
                    }
                }
            }
        }</code>
    
    
    
    
    To fix it, I had to change it to the following:
    private async Task<ICommunicationAttemptOutcome<T>> SendAsync<T>(
        IRequestConfiguration configuration,
        HttpRequestMessage message)
        where T : IResponseMessage, new()
    {
        if (configuration.MetadataElements != null)
            foreach (var metadataElement in configuration.MetadataElements)
                message.Headers.Add(metadataElement.Name, metadataElement.Value);

    HttpResponseMessage response = null;
    T responseMessage;
    try
    {
        try
        {
            using (var wrapper = httpClientWrapperFactory.Create(configuration))
                response = await wrapper.SendAsync(message);
        }
        catch (Exception e)
        {
            return configuration.FailedCommunicationAttemptMapper.MapException&lt;T&gt;(e);
        }
    
        var contentString = await response.Content.ReadAsStringAsync();
    
        if (configuration.ResponseSuccessDeterminer.IsSuccessful&lt;T&gt;(response.IsSuccessStatusCode, contentString))
        {
            if (configuration.SuccessfulResponseProcessor != null)
                configuration.SuccessfulResponseProcessor.Process&lt;T&gt;(contentString);
    
            if (response.StatusCode == HttpStatusCode.NoContent)
                return new CommunicationAttemptOutcome&lt;T&gt;(new T());
    
            if (configuration.DataSerialiser.TryDeserialise(contentString, out responseMessage))
                return new CommunicationAttemptOutcome&lt;T&gt;(responseMessage);
    
            return new CommunicationAttemptOutcome&lt;T&gt;(responseMessage);
        }
    
        return configuration.FailedCommunicationAttemptMapper.MapUnsuccessfulResponse&lt;T&gt;(contentString);
    }
    finally
    {
        if (response != null)
        {
            try
            {
                logger.Trace(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), &quot;Disposing of HttpResponseMessage&quot;);
                response.Dispose();
                logger.Info(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), &quot;Disposed of HttpResponseMessage&quot;);
            }
            catch (Exception e)
            {
                logger.Warn(&quot;SendAsync&quot;, typeof(HttpClientRequestWrapper), e, &quot;Unable to dispose of HttpResponseMessage&quot;);
            }
        }
    }
    
    
    
    
    }

    It really didn't like the

    T responseMessage
    
    in that block.

    And it didn't like

    var logger = serviceLocator.Resolve<ILogger>();
    
    in the finally block

    (apologies for the code formatting above, I couldn't get it to not split the code up!)

  7. Support Staff 7 Posted by PostSharp Techn... on 18 Dec, 2017 05:23 PM

    PostSharp Technologies's Avatar

    Hello,

    thanks a lot for the second case.

    Best regards,
    Daniel

  8. Support Staff 8 Posted by PostSharp Techn... on 03 Jan, 2018 01:15 PM

    PostSharp Technologies's Avatar

    Hello,

    We're closing the ticket for now as the bug has been internally filed as issue #15750. We will contact you as soon as the bug fix has been released.

    For more details on our support policies and prioritization of bug fixes, please visit https://www.postsharp.net/support/policies

    Thanks,
    PostSharp Team

  9. PostSharp Technologies closed this discussion on 03 Jan, 2018 01:15 PM.

  10. PostSharp Technologies re-opened this discussion on 27 Jan, 2018 08:02 PM

  11. Support Staff 9 Posted by PostSharp Techn... on 27 Jan, 2018 08:02 PM

    PostSharp Technologies's Avatar

    Hello,

    the bug #15750 has been fixed in the current release of PostSharp 5.0.44.
    Should you need further help with this issue, don't hesitate to re-open this discussion.

    -alex

  12. PostSharp Technologies closed this discussion on 27 Jan, 2018 08:03 PM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac