{"action":"closed","issue":{"url":"https://api.github.com/repos/aspnet/Mvc/issues/8579","repository_url":"https://api.github.com/repos/aspnet/Mvc","labels_url":"https://api.github.com/repos/aspnet/Mvc/issues/8579/labels{/name}","comments_url":"https://api.github.com/repos/aspnet/Mvc/issues/8579/comments","events_url":"https://api.github.com/repos/aspnet/Mvc/issues/8579/events","html_url":"https://github.com/aspnet/Mvc/issues/8579","id":368617805,"node_id":"MDU6SXNzdWUzNjg2MTc4MDU=","number":8579,"title":"TypeFilterAttribute does not honor IsReusable on initial requests if simultaneous","user":{"login":"ti82","id":8225564,"node_id":"MDQ6VXNlcjgyMjU1NjQ=","avatar_url":"https://avatars3.githubusercontent.com/u/8225564?v=4","gravatar_id":"","url":"https://api.github.com/users/ti82","html_url":"https://github.com/ti82","followers_url":"https://api.github.com/users/ti82/followers","following_url":"https://api.github.com/users/ti82/following{/other_user}","gists_url":"https://api.github.com/users/ti82/gists{/gist_id}","starred_url":"https://api.github.com/users/ti82/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ti82/subscriptions","organizations_url":"https://api.github.com/users/ti82/orgs","repos_url":"https://api.github.com/users/ti82/repos","events_url":"https://api.github.com/users/ti82/events{/privacy}","received_events_url":"https://api.github.com/users/ti82/received_events","type":"User","site_admin":false},"labels":[{"id":70264664,"node_id":"MDU6TGFiZWw3MDI2NDY2NA==","url":"https://api.github.com/repos/aspnet/Mvc/labels/question","name":"question","color":"cc317c","default":true}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-10-10T11:21:33Z","updated_at":"2018-10-12T17:08:42Z","closed_at":"2018-10-12T17:08:42Z","author_association":"NONE","body":"Bug\r\n\r\nHere is my ConnectionLimitAttribute which I use to limit connections to a controller action, and return a 503 if the limit has been reached:\r\n\r\n```\r\n[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = false)]\r\n    public class ConnectionLimitAttribute : TypeFilterAttribute\r\n    {\r\n        /// <summary>\r\n        /// Creates a new instance of the <see cref=\"ConnectionLimitAttribute\"/> class.\r\n        /// </summary>\r\n        public ConnectionLimitAttribute(int limit) : base(typeof(ConnectionLimitImpl))\r\n        {\r\n            // Pass the connection limit argument to the implementation\r\n            this.Arguments = new object[] { limit };\r\n            this.IsReusable = true;\r\n        }\r\n\r\n        class ConnectionLimitImpl : IResourceFilter\r\n        {\r\n            private readonly ILogger<ConnectionLimitAttribute> logger;\r\n            private readonly int limit;\r\n            private readonly string id;\r\n\r\n            private static object safe = new object();\r\n\r\n            private int connectionCount;\r\n            \r\n            /// <summary>\r\n            /// Creates a new instance of the <see cref=\"ConnectionLimitImpl\"/> class.\r\n            /// </summary>\r\n            /// <param name=\"limit\">The number of simultaneous connections to allow.</param>\r\n            public ConnectionLimitImpl(int limit, ILoggerFactory loggerFactory)\r\n            {\r\n                this.id = Guid.NewGuid().ToString(\"D\").ToUpperInvariant();\r\n\r\n                this.limit = limit;\r\n                this.logger = loggerFactory.CreateLogger<ConnectionLimitAttribute>();\r\n            }\r\n\r\n            /// <summary>\r\n            /// Implementation of the <see cref=\"IResourceFilter\"/> interface.\r\n            /// </summary>\r\n            public void OnResourceExecuted(ResourceExecutedContext context)\r\n            {\r\n                lock (safe)\r\n                {\r\n                    // The resource has been executed so the connectionCount can be decremented\r\n                    this.logger.LogInformation($\"REQUEST {context.HttpContext.TraceIdentifier} CONNECTION {this.id} RELEASED\");\r\n                    Interlocked.Decrement(ref this.connectionCount);\r\n                }\r\n            }\r\n\r\n            /// <summary>\r\n            /// Implementation of the <see cref=\"IResourceFilter\"/> interface.\r\n            /// </summary>\r\n            public void OnResourceExecuting(ResourceExecutingContext context)\r\n            {\r\n                // Need to lock around full conditional because two very close requests could\r\n                // get past the CompareExchange and into the Increment\r\n                lock (safe)\r\n                {\r\n                    // Check the connectionCount against the limit and respond appropriately\r\n                    if (limit == Interlocked.CompareExchange(ref this.connectionCount, this.limit, this.limit))\r\n                    {\r\n                        this.logger.LogInformation($\"REQUEST {context.HttpContext.TraceIdentifier} CONNECTION {this.id} REJECTED\");\r\n                        // Indicate that the connection limit was exceeded with a HTTP 503 response\r\n                        context.Result = new StatusCodeResult(StatusCodes.Status503ServiceUnavailable);\r\n                    }\r\n                    else\r\n                    {\r\n                        // Increment the connectionCount and let the pipeline continue\r\n                        Interlocked.Increment(ref this.connectionCount);\r\n                        this.logger.LogInformation($\"REQUEST {context.HttpContext.TraceIdentifier} CONNECTION {this.id} ACQUIRED\");\r\n                    }\r\n                }\r\n            }\r\n        }        \r\n    }\r\n```\r\n\r\nMy app has a single controller method, where this attribute is set to 1. Note the use of IsReusable = true in the Impl constructor. If, when the service is first started, there are several connections rapidly, it does not reuse the filter initially:\r\n\r\ninfo: ConnectionLimitAttribute[0]\r\n      REQUEST 0HLHEK2IQQTF9:00000001 CONNECTION 9AABD770-14D1-40F6-8930-9C3EBB5F9A8C ACQUIRED\r\ninfo: ConnectionLimitAttribute[0]\r\n      REQUEST 0HLHEK2IQQTF8:00000001 CONNECTION 27903C64-5CF4-4E7F-9B06-D341135918FF ACQUIRED\r\ninfo: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]\r\n      Request starting HTTP/1.1 POST http://127.0.0.1:5020/alg/run multipart/form-data; boundary=\"26fbf333-5412-43eb-b739-9c46d83447c8\" 1756239\r\ninfo: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]\r\n      Request starting HTTP/1.1 POST http://127.0.0.1:5020/alg/run multipart/form-data; boundary=\"c6e6a6bf-0db7-4c25-8931-f2be9c7084ef\" 1756239\r\ninfo: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[1]\r\n      Route matched with {action = \"PostAsync\", controller = \"Algorithm\"}. Executing action AlgorithmController.PostAsync (AlgorithmHostService)\r\ninfo: ConnectionLimitAttribute[0]\r\n      REQUEST 0HLHEK2IQQTFA:00000001 CONNECTION 9AABD770-14D1-40F6-8930-9C3EBB5F9A8C REJECTED\r\ninfo: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]\r\n      Request starting HTTP/1.1 POST http://127.0.0.1:5020/alg/run multipart/form-data; boundary=\"60cb7f2c-f4ab-4b2a-b3a0-33ebf66cb7f5\" 1756239\r\ninfo: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[1]\r\n      Route matched with {action = \"PostAsync\", controller = \"Algorithm\"}. Executing action AlgorithmController.PostAsync (AlgorithmHostService)\r\ninfo: Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker[1]\r\n      Route matched with {action = \"PostAsync\", controller = \"Algorithm\"}. Executing action AlgorithmController.PostAsync (AlgorithmHostService)\r\ninfo: ConnectionLimitAttribute[0]\r\n      REQUEST 0HLHEK2IQQTFC:00000001 CONNECTION 9AABD770-14D1-40F6-8930-9C3EBB5F9A8C REJECTED\r\ninfo: ConnectionLimitAttribute[0]\r\n      REQUEST 0HLHEK2IQQTFB:00000001 CONNECTION 9AABD770-14D1-40F6-8930-9C3EBB5F9A8C REJECTED\r\n\r\nNowhere in the documentation at https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/filters?view=aspnetcore-2.1 is this concurrency issue discussed, hence I did not expect this behavior."}}
{"actor":{"display_login":"mkArtakMSFT"}}