Giter VIP home page Giter VIP logo

Comments (7)

alexmg avatar alexmg commented on July 25, 2024

Thanks for providing the sample solution. It's always appreciated when the effort has been taken to provide a reproduction.

If you remove the RegisterCallback handler method your Application_Start() you should find everything works fine.

It is attempting to resolve the HttpRequestMessage in the Preparing event handler which is causing a circular dependency.

Circular component dependency detected: AutofacRequestMessageDisposed.Controllers.ValuesController -> λ:System.Net.Http.HttpRequestMessage -> λ:System.Net.Http.HttpRequestMessage.

You should note that this callback is invoked for every service that is resolved. That includes the HttpRequestMessage itself and anything the framework resolves during any part of the request lifecycle. Performing the resolve operation in that callback will lead to all sorts of exceptions. I assume someone already discovered this based on the fact there are empty catch blocks around the code. That code may have been added to debug something but having it there will only cause further issues.

from autofac.webapi.

lillo42 avatar lillo42 commented on July 25, 2024

@alexmg Thank you, even if I add a if to not do that tracing for HttpRequestMessage the problem continue :(

 builder.RegisterCallback(callback =>
            {
                callback.Registered += (sender, args) =>
                {
                    if (args.ComponentRegistration.Activator.LimitType == typeof(HttpRequestMessage))
                    {
                        return;
                    }

                    args.ComponentRegistration.Preparing += (sender2, arg2) =>
                    {
                        try
                        {
                            var message = arg2.Context.Resolve<HttpRequestMessage>();
                            if (!message.Headers.Contains("Preparing"))
                            {
                                message.Headers.Add("Preparing", DateTime.Now.ToString());
                            }
                            
                        }
                        catch (Exception)
                        {
                        }
                    };

                    args.ComponentRegistration.Activated += (sender2, arg2) =>
                    {
                        try
                        {
                            var message = arg2.Context.Resolve<HttpRequestMessage>();
                            if (!message.Headers.Contains("Activated"))
                            {
                                message.Headers.Add("Activated", DateTime.Now.ToString());
                            }

                        }
                        catch (Exception)
                        {
                        }
                    };
                };
            });

from autofac.webapi.

alexmg avatar alexmg commented on July 25, 2024

The controller will be resolved before the HttpRequestMessage dependency so that will not be enough to stop the circular reference. Try commenting out the entire RegisterCallback and it should be fine. That is not the right place to interact with the message instance.

from autofac.webapi.

lillo42 avatar lillo42 commented on July 25, 2024

@alexmg Ok, just one more question, if the problem is circular reference why when I mark HttpRequestMessage with ExternallyOwned start to work?

public class WebApiApplication : System.Web.HttpApplication
    {
        protected void Application_Start()
        {
            var builder = new ContainerBuilder();

            // Get your HttpConfiguration.
            var config = GlobalConfiguration.Configuration;

            builder.RegisterCallback( ... );
            ....

            // builder.RegisterHttpRequestMessage(config);
          // Autofac code
            builder.Register(c => HttpRequestMessageProvider.Current)
               .InstancePerRequest()
                .ExternallyOwned(); // <--- this was added
               ...
        }
    }

    // Copy from Autofac source code
    /// <summary>
    /// A delegating handler that updates the current dependency scope
    /// with the current <see cref="HttpRequestMessage"/>.
    /// </summary>
    public class CurrentRequestHandler : DelegatingHandler
    {
        /// <summary>
        /// Sends an HTTP request to the inner handler to send to the server as an asynchronous operation.
        /// </summary>
        /// <param name="request">The HTTP request message to send to the server.</param>
        /// <param name="cancellationToken">A cancellation token to cancel operation.</param>
        /// <returns>
        /// Returns <see cref="System.Threading.Tasks.Task{T}" />. The task object representing the asynchronous operation.
        /// </returns>
        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            UpdateScopeWithHttpRequestMessage(request);

            return base.SendAsync(request, cancellationToken);
        }

        /// <summary>
        /// Updates the current dependency scope with current HTTP request message.
        /// </summary>
        /// <param name="request">The HTTP request message.</param>
        internal static void UpdateScopeWithHttpRequestMessage(HttpRequestMessage request)
        {
            HttpRequestMessageProvider.Current = request;
        }
    }

    public static class HttpRequestMessageProvider
    {
        private static readonly string Key = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture).Substring(0, 12);

        internal static HttpRequestMessage Current
        {
            get
            {
                var wrapper = (HttpRequestMessageWrapper)CallContext.LogicalGetData(Key);
                return wrapper?.Message;
            }

            set
            {
                var wrapper = value == null ? null : new HttpRequestMessageWrapper(value);
                CallContext.LogicalSetData(Key, wrapper);
            }
        }

        [Serializable]
        private sealed class HttpRequestMessageWrapper : MarshalByRefObject
        {
            [NonSerialized]
            [SuppressMessage("SA1401", "SA1401", Justification = "Field is only used during testing.")]
            internal readonly HttpRequestMessage Message;

            internal HttpRequestMessageWrapper(HttpRequestMessage message)
            {
                Message = message;
            }
        }
    }

from autofac.webapi.

alexmg avatar alexmg commented on July 25, 2024

There were two exception being thrown. The first exception was caused by the circular dependency and the second was a result of resolving the HttpRequestMessage from the root lifetime scope and not the request lifetime scope (it searches up the chain of lifetime scopes). Both exceptions stem from resolving the HttpRequestMessage in the Preparing handler.

A key reason for the request lifetime scoping is to ensure that services are not used outside of the request lifetime. This case is an interesting one though. Because we are not actually responsible for creating the HttpRequestMessage in the first place, it could be said that we should also not take responsibility for its disposal, because doing so could result in it being disposed before the Web API framework intended.

I see you have a PR open to make the HttpRequestMessage service ExternallyOwned. I think that leaving Web API to dispose the HttpRequestMessage is the right thing to do for this service. You should still be careful about what you resolve in those handlers because an exception will still be thrown due to the circular dependency, but that could be considered a separate issue to who is responsible for disposing the HttpRequestMessage.

It's been a long time since I have worked with Web API so it would be good to get a second opinion, but I think the change in your PR makes sense given we did not create the instance in the first place. /cc @tillig

from autofac.webapi.

tillig avatar tillig commented on July 25, 2024

I think marking HttpRequestMessage as ExternallyOwned is reasonable. Neither we nor the developer created it; we're merely including it for convenience and probably shouldn't be taking ownership.

from autofac.webapi.

alexmg avatar alexmg commented on July 25, 2024

Thanks for the PR @lillo42.

I have released 6.0.1 with the fix. :shipit:

from autofac.webapi.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.