{"action":"opened","number":8332,"pull_request":{"url":"https://api.github.com/repos/netty/netty/pulls/8332","id":218757664,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE4NzU3NjY0","html_url":"https://github.com/netty/netty/pull/8332","diff_url":"https://github.com/netty/netty/pull/8332.diff","patch_url":"https://github.com/netty/netty/pull/8332.patch","issue_url":"https://api.github.com/repos/netty/netty/issues/8332","number":8332,"state":"open","locked":false,"title":"Notify http2 error handler before closeStreamLocal on HEADERS write failure","user":{"login":"ejona86","id":2811396,"node_id":"MDQ6VXNlcjI4MTEzOTY=","avatar_url":"https://avatars2.githubusercontent.com/u/2811396?v=4","gravatar_id":"","url":"https://api.github.com/users/ejona86","html_url":"https://github.com/ejona86","followers_url":"https://api.github.com/users/ejona86/followers","following_url":"https://api.github.com/users/ejona86/following{/other_user}","gists_url":"https://api.github.com/users/ejona86/gists{/gist_id}","starred_url":"https://api.github.com/users/ejona86/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ejona86/subscriptions","organizations_url":"https://api.github.com/users/ejona86/orgs","repos_url":"https://api.github.com/users/ejona86/repos","events_url":"https://api.github.com/users/ejona86/events{/privacy}","received_events_url":"https://api.github.com/users/ejona86/received_events","type":"User","site_admin":false},"body":"Motivation:\r\n\r\nWhen writing an HTTP/2 HEADERS with END_STREAM=1, the application expects\r\nthe stream to be closed afterward. However, the write can fail locally\r\ndue to HPACK encoder and similar. When that happens we need to make sure\r\nto issue a RST_STREAM otherwise the stream can be closed locally but\r\norphaned remotely. The RST_STREAM is typically handled by\r\nHttp2ConnectionHandler.onStreamError, which will only send a RST_STREAM\r\nif that stream still exists locally.\r\n\r\nThere are two possible flows for trailers, one handled immediately and\r\none going through the flow controller. Previously they behaved\r\ndifferently, with the immedate code calling the error handler after\r\nclosing the stream. The immediate code also used a listener for calling\r\ncloseStreamLocal while the flow controlled code did so immediately after\r\nthe write.\r\n\r\nThe two code paths also differed in their VoidChannelPromise handling,\r\nbut both were broken. The immediate code path called unvoid() only if\r\nEND_STREAM=1, however it could always potentially add a listener via\r\nnotifyLifecycleManagerOnError(). And the flow controlled code path\r\nunvoided incorrectly, changing the promise completion behavior. It also\r\npassed the wrong promise to closeStreamLocal() in FlowControlledBase.\r\n\r\nModifications:\r\n\r\nMove closeStreamLocal handling after calls to onError. This is the\r\nprimary change.\r\n\r\nNow call closeStreamLocal immediately instead of when the future\r\ncompletes. This is the more likely correct behavior as it matches that\r\nof DATA frames.\r\n\r\nFix all the VoidChannelPromise handling.\r\n\r\nResult:\r\n\r\nHttp2ConnectionHandler.onStreamError sees the same state as the remote\r\nand issues a RST_STREAM, properly cleaning up the stream.\r\n\r\n------\r\n\r\nCC @normanmaurer, @Scottmitch\r\n\r\nThis issue impacts trailers and headers-only responses with malformed headers.\r\nThis was breaking a new gRPC test I wrote that tested MAX_HEADER_LIST_SIZE\r\nbehavior and caused the client RPC to time out because the server never sent a\r\nresponse.\r\n\r\nThis does raise an oddity that we will closeStreamLocal() even if the END_STREAM=1\r\nframe failed. That risks the local and remote states becoming out-of-sync and today\r\nworks because of Http2ConnectionHandler's behavior. However, given\r\nHttp2ConnectionHandler's current behavior it's not necessary at all to closeStreamLocal()\r\non failure; the rstStream will do that. Any change there seems like it should be a separate\r\ndiscussion though.","created_at":"2018-09-27T19:06:40Z","updated_at":"2018-09-27T19:06:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/netty/netty/pulls/8332/commits","review_comments_url":"https://api.github.com/repos/netty/netty/pulls/8332/comments","review_comment_url":"https://api.github.com/repos/netty/netty/pulls/comments{/number}","comments_url":"https://api.github.com/repos/netty/netty/issues/8332/comments","statuses_url":"https://api.github.com/repos/netty/netty/statuses/d65453e065ddda5364ae7ae6043281f90f4e39aa","head":{"label":"ejona86:trailers-encoding-error-order","ref":"trailers-encoding-error-order","sha":"d65453e065ddda5364ae7ae6043281f90f4e39aa","user":{"login":"ejona86","id":2811396,"node_id":"MDQ6VXNlcjI4MTEzOTY=","avatar_url":"https://avatars2.githubusercontent.com/u/2811396?v=4","gravatar_id":"","url":"https://api.github.com/users/ejona86","html_url":"https://github.com/ejona86","followers_url":"https://api.github.com/users/ejona86/followers","following_url":"https://api.github.com/users/ejona86/following{/other_user}","gists_url":"https://api.github.com/users/ejona86/gists{/gist_id}","starred_url":"https://api.github.com/users/ejona86/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ejona86/subscriptions","organizations_url":"https://api.github.com/users/ejona86/orgs","repos_url":"https://api.github.com/users/ejona86/repos","events_url":"https://api.github.com/users/ejona86/events{/privacy}","received_events_url":"https://api.github.com/users/ejona86/received_events","type":"User","site_admin":false},"repo":{"id":32953128,"node_id":"MDEwOlJlcG9zaXRvcnkzMjk1MzEyOA==","name":"netty","full_name":"ejona86/netty","private":false,"owner":{"login":"ejona86","id":2811396,"node_id":"MDQ6VXNlcjI4MTEzOTY=","avatar_url":"https://avatars2.githubusercontent.com/u/2811396?v=4","gravatar_id":"","url":"https://api.github.com/users/ejona86","html_url":"https://github.com/ejona86","followers_url":"https://api.github.com/users/ejona86/followers","following_url":"https://api.github.com/users/ejona86/following{/other_user}","gists_url":"https://api.github.com/users/ejona86/gists{/gist_id}","starred_url":"https://api.github.com/users/ejona86/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ejona86/subscriptions","organizations_url":"https://api.github.com/users/ejona86/orgs","repos_url":"https://api.github.com/users/ejona86/repos","events_url":"https://api.github.com/users/ejona86/events{/privacy}","received_events_url":"https://api.github.com/users/ejona86/received_events","type":"User","site_admin":false},"html_url":"https://github.com/ejona86/netty","description":"Netty project - an event-driven asynchronous network application framework","fork":true,"url":"https://api.github.com/repos/ejona86/netty","forks_url":"https://api.github.com/repos/ejona86/netty/forks","keys_url":"https://api.github.com/repos/ejona86/netty/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ejona86/netty/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ejona86/netty/teams","hooks_url":"https://api.github.com/repos/ejona86/netty/hooks","issue_events_url":"https://api.github.com/repos/ejona86/netty/issues/events{/number}","events_url":"https://api.github.com/repos/ejona86/netty/events","assignees_url":"https://api.github.com/repos/ejona86/netty/assignees{/user}","branches_url":"https://api.github.com/repos/ejona86/netty/branches{/branch}","tags_url":"https://api.github.com/repos/ejona86/netty/tags","blobs_url":"https://api.github.com/repos/ejona86/netty/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ejona86/netty/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ejona86/netty/git/refs{/sha}","trees_url":"https://api.github.com/repos/ejona86/netty/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ejona86/netty/statuses/{sha}","languages_url":"https://api.github.com/repos/ejona86/netty/languages","stargazers_url":"https://api.github.com/repos/ejona86/netty/stargazers","contributors_url":"https://api.github.com/repos/ejona86/netty/contributors","subscribers_url":"https://api.github.com/repos/ejona86/netty/subscribers","subscription_url":"https://api.github.com/repos/ejona86/netty/subscription","commits_url":"https://api.github.com/repos/ejona86/netty/commits{/sha}","git_commits_url":"https://api.github.com/repos/ejona86/netty/git/commits{/sha}","comments_url":"https://api.github.com/repos/ejona86/netty/comments{/number}","issue_comment_url":"https://api.github.com/repos/ejona86/netty/issues/comments{/number}","contents_url":"https://api.github.com/repos/ejona86/netty/contents/{+path}","compare_url":"https://api.github.com/repos/ejona86/netty/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ejona86/netty/merges","archive_url":"https://api.github.com/repos/ejona86/netty/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ejona86/netty/downloads","issues_url":"https://api.github.com/repos/ejona86/netty/issues{/number}","pulls_url":"https://api.github.com/repos/ejona86/netty/pulls{/number}","milestones_url":"https://api.github.com/repos/ejona86/netty/milestones{/number}","notifications_url":"https://api.github.com/repos/ejona86/netty/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ejona86/netty/labels{/name}","releases_url":"https://api.github.com/repos/ejona86/netty/releases{/id}","deployments_url":"https://api.github.com/repos/ejona86/netty/deployments","created_at":"2015-03-26T21:34:39Z","updated_at":"2018-09-26T22:32:29Z","pushed_at":"2018-09-27T18:59:54Z","git_url":"git://github.com/ejona86/netty.git","ssh_url":"git@github.com:ejona86/netty.git","clone_url":"https://github.com/ejona86/netty.git","svn_url":"https://github.com/ejona86/netty","homepage":"http://netty.io/","size":46804,"stargazers_count":1,"watchers_count":1,"language":"Java","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":0,"open_issues":0,"watchers":1,"default_branch":"4.1"}},"base":{"label":"netty:4.1","ref":"4.1","sha":"2d7cb47eddc79c4b43a1fb3996e8cb8bde15bff3","user":{"login":"netty","id":473791,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3Mzc5MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/473791?v=4","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"repo":{"id":1064563,"node_id":"MDEwOlJlcG9zaXRvcnkxMDY0NTYz","name":"netty","full_name":"netty/netty","private":false,"owner":{"login":"netty","id":473791,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3Mzc5MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/473791?v=4","gravatar_id":"","url":"https://api.github.com/users/netty","html_url":"https://github.com/netty","followers_url":"https://api.github.com/users/netty/followers","following_url":"https://api.github.com/users/netty/following{/other_user}","gists_url":"https://api.github.com/users/netty/gists{/gist_id}","starred_url":"https://api.github.com/users/netty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netty/subscriptions","organizations_url":"https://api.github.com/users/netty/orgs","repos_url":"https://api.github.com/users/netty/repos","events_url":"https://api.github.com/users/netty/events{/privacy}","received_events_url":"https://api.github.com/users/netty/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/netty/netty","description":"Netty project - an event-driven asynchronous network application framework","fork":false,"url":"https://api.github.com/repos/netty/netty","forks_url":"https://api.github.com/repos/netty/netty/forks","keys_url":"https://api.github.com/repos/netty/netty/keys{/key_id}","collaborators_url":"https://api.github.com/repos/netty/netty/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/netty/netty/teams","hooks_url":"https://api.github.com/repos/netty/netty/hooks","issue_events_url":"https://api.github.com/repos/netty/netty/issues/events{/number}","events_url":"https://api.github.com/repos/netty/netty/events","assignees_url":"https://api.github.com/repos/netty/netty/assignees{/user}","branches_url":"https://api.github.com/repos/netty/netty/branches{/branch}","tags_url":"https://api.github.com/repos/netty/netty/tags","blobs_url":"https://api.github.com/repos/netty/netty/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/netty/netty/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/netty/netty/git/refs{/sha}","trees_url":"https://api.github.com/repos/netty/netty/git/trees{/sha}","statuses_url":"https://api.github.com/repos/netty/netty/statuses/{sha}","languages_url":"https://api.github.com/repos/netty/netty/languages","stargazers_url":"https://api.github.com/repos/netty/netty/stargazers","contributors_url":"https://api.github.com/repos/netty/netty/contributors","subscribers_url":"https://api.github.com/repos/netty/netty/subscribers","subscription_url":"https://api.github.com/repos/netty/netty/subscription","commits_url":"https://api.github.com/repos/netty/netty/commits{/sha}","git_commits_url":"https://api.github.com/repos/netty/netty/git/commits{/sha}","comments_url":"https://api.github.com/repos/netty/netty/comments{/number}","issue_comment_url":"https://api.github.com/repos/netty/netty/issues/comments{/number}","contents_url":"https://api.github.com/repos/netty/netty/contents/{+path}","compare_url":"https://api.github.com/repos/netty/netty/compare/{base}...{head}","merges_url":"https://api.github.com/repos/netty/netty/merges","archive_url":"https://api.github.com/repos/netty/netty/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/netty/netty/downloads","issues_url":"https://api.github.com/repos/netty/netty/issues{/number}","pulls_url":"https://api.github.com/repos/netty/netty/pulls{/number}","milestones_url":"https://api.github.com/repos/netty/netty/milestones{/number}","notifications_url":"https://api.github.com/repos/netty/netty/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/netty/netty/labels{/name}","releases_url":"https://api.github.com/repos/netty/netty/releases{/id}","deployments_url":"https://api.github.com/repos/netty/netty/deployments","created_at":"2010-11-09T09:22:21Z","updated_at":"2018-09-27T19:01:18Z","pushed_at":"2018-09-27T19:01:04Z","git_url":"git://github.com/netty/netty.git","ssh_url":"git@github.com:netty/netty.git","clone_url":"https://github.com/netty/netty.git","svn_url":"https://github.com/netty/netty","homepage":"http://netty.io/","size":52375,"stargazers_count":15809,"watchers_count":15809,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":6943,"mirror_url":null,"archived":false,"open_issues_count":388,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":6943,"open_issues":388,"watchers":15809,"default_branch":"4.1"}},"_links":{"self":{"href":"https://api.github.com/repos/netty/netty/pulls/8332"},"html":{"href":"https://github.com/netty/netty/pull/8332"},"issue":{"href":"https://api.github.com/repos/netty/netty/issues/8332"},"comments":{"href":"https://api.github.com/repos/netty/netty/issues/8332/comments"},"review_comments":{"href":"https://api.github.com/repos/netty/netty/pulls/8332/comments"},"review_comment":{"href":"https://api.github.com/repos/netty/netty/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/netty/netty/pulls/8332/commits"},"statuses":{"href":"https://api.github.com/repos/netty/netty/statuses/d65453e065ddda5364ae7ae6043281f90f4e39aa"}},"author_association":"CONTRIBUTOR","merged":false,"mergeable":null,"rebaseable":null,"mergeable_state":"unknown","merged_by":null,"comments":0,"review_comments":0,"maintainer_can_modify":true,"commits":1,"additions":43,"deletions":14,"changed_files":2}}
{"actor":{"display_login":"ejona86"}}