{"action":"created","comment":{"url":"https://api.github.com/repos/servo/ipc-channel/pulls/comments/53572566","id":53572566,"diff_hunk":"@@ -137,57 +137,65 @@ impl UnixSender {\n         }\n \n         unsafe {\n-            let cmsg_length =\n-                (channels.len() + shared_memory_regions.len()) * mem::size_of::<c_int>();\n-            let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length as size_t)) as *mut cmsghdr;\n-            (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length as size_t);\n-            (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET;\n-            (*cmsg_buffer).cmsg_type = SCM_RIGHTS;\n-\n-            let mut fds = Vec::new();\n-            for channel in channels.iter() {\n-                fds.push(channel.fd());\n-            }\n-            for shared_memory_region in shared_memory_regions.iter() {\n-                fds.push(shared_memory_region.fd);\n-            }\n-            ptr::copy_nonoverlapping(fds.as_ptr(),\n-                                     cmsg_buffer.offset(1) as *mut _ as *mut c_int,\n-                                     fds.len());\n-            let mut cmsg_padding_ptr =\n-                (cmsg_buffer.offset(1) as *mut _ as *mut c_int).offset(fds.len() as isize);\n-            let cmsg_end =\n-                (cmsg_buffer as *mut _ as *mut u8).offset(CMSG_SPACE(cmsg_length as size_t) as\n-                                                          isize);\n-            while (cmsg_padding_ptr as *mut u8) < cmsg_end {\n-                *cmsg_padding_ptr = *DEV_NULL;\n-                cmsg_padding_ptr = cmsg_padding_ptr.offset(1);\n-            }\n+            let construct_header = |channels: &Vec<UnixChannel>, data_buffer: &Vec<u8>| {\n+                let cmsg_length =\n+                    (channels.len() + shared_memory_regions.len()) * mem::size_of::<c_int>();\n+                let cmsg_buffer = libc::malloc(CMSG_SPACE(cmsg_length as size_t)) as *mut cmsghdr;\n+                (*cmsg_buffer).cmsg_len = CMSG_LEN(cmsg_length as size_t);\n+                (*cmsg_buffer).cmsg_level = libc::SOL_SOCKET;\n+                (*cmsg_buffer).cmsg_type = SCM_RIGHTS;\n+\n+                let mut fds = Vec::new();\n+                for channel in channels.iter() {\n+                    fds.push(channel.fd());\n+                }\n+                for shared_memory_region in shared_memory_regions.iter() {\n+                    fds.push(shared_memory_region.fd);\n+                }\n+                ptr::copy_nonoverlapping(fds.as_ptr(),\n+                                         cmsg_buffer.offset(1) as *mut _ as *mut c_int,\n+                                         fds.len());\n+                let mut cmsg_padding_ptr =\n+                    (cmsg_buffer.offset(1) as *mut _ as *mut c_int).offset(fds.len() as isize);\n+                let cmsg_end =\n+                    (cmsg_buffer as *mut _ as *mut u8).offset(CMSG_SPACE(cmsg_length as size_t) as\n+                                                              isize);\n+                while (cmsg_padding_ptr as *mut u8) < cmsg_end {\n+                    *cmsg_padding_ptr = *DEV_NULL;\n+                    cmsg_padding_ptr = cmsg_padding_ptr.offset(1);\n+                }\n \n-            let mut iovec = iovec {\n-                iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char,\n-                iov_len: data_buffer.len() as size_t,\n-            };\n+                // Put this on the heap so address remains stable across function return.\n+                let mut iovec = Box::new(iovec {\n+                    iov_base: data_buffer.as_ptr() as *const c_char as *mut c_char,\n+                    iov_len: data_buffer.len() as size_t,\n+                });\n+\n+                let msghdr = msghdr {\n+                    msg_name: ptr::null_mut(),\n+                    msg_namelen: 0,\n+                    msg_iov: &mut *iovec,\n+                    msg_iovlen: 1,\n+                    msg_control: cmsg_buffer as *mut c_void,\n+                    msg_controllen: CMSG_SPACE(cmsg_length as size_t),\n+                    msg_flags: 0,\n+                };\n \n-            let msghdr = msghdr {\n-                msg_name: ptr::null_mut(),\n-                msg_namelen: 0,\n-                msg_iov: &mut iovec,\n-                msg_iovlen: 1,\n-                msg_control: cmsg_buffer as *mut c_void,\n-                msg_controllen: CMSG_SPACE(cmsg_length as size_t),\n-                msg_flags: 0,\n+                // Be sure to always return iovec -- whether the caller uses it or not --\n+                // to prevent premature deallocation!\n+                (msghdr, iovec)","path":"platform/linux/mod.rs","position":97,"original_position":97,"commit_id":"813c62d79328d75b30f755ec2927435436e615b1","original_commit_id":"813c62d79328d75b30f755ec2927435436e615b1","user":{"login":"ecoal95","id":1323194,"avatar_url":"https://avatars.githubusercontent.com/u/1323194?v=3","gravatar_id":"","url":"https://api.github.com/users/ecoal95","html_url":"https://github.com/ecoal95","followers_url":"https://api.github.com/users/ecoal95/followers","following_url":"https://api.github.com/users/ecoal95/following{/other_user}","gists_url":"https://api.github.com/users/ecoal95/gists{/gist_id}","starred_url":"https://api.github.com/users/ecoal95/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ecoal95/subscriptions","organizations_url":"https://api.github.com/users/ecoal95/orgs","repos_url":"https://api.github.com/users/ecoal95/repos","events_url":"https://api.github.com/users/ecoal95/events{/privacy}","received_events_url":"https://api.github.com/users/ecoal95/received_events","type":"User","site_admin":false},"body":"Whoops, forget that","created_at":"2016-02-21T20:19:50Z","updated_at":"2016-02-21T20:19:50Z","html_url":"https://github.com/servo/ipc-channel/pull/38#discussion_r53572566","pull_request_url":"https://api.github.com/repos/servo/ipc-channel/pulls/38","_links":{"self":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/comments/53572566"},"html":{"href":"https://github.com/servo/ipc-channel/pull/38#discussion_r53572566"},"pull_request":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/38"}}},"pull_request":{"url":"https://api.github.com/repos/servo/ipc-channel/pulls/38","id":60097700,"html_url":"https://github.com/servo/ipc-channel/pull/38","diff_url":"https://github.com/servo/ipc-channel/pull/38.diff","patch_url":"https://github.com/servo/ipc-channel/pull/38.patch","issue_url":"https://api.github.com/repos/servo/ipc-channel/issues/38","number":38,"state":"open","locked":false,"title":"Fix concurrent large transfers by using dedicated channel","user":{"login":"antrik","id":19555,"avatar_url":"https://avatars.githubusercontent.com/u/19555?v=3","gravatar_id":"","url":"https://api.github.com/users/antrik","html_url":"https://github.com/antrik","followers_url":"https://api.github.com/users/antrik/followers","following_url":"https://api.github.com/users/antrik/following{/other_user}","gists_url":"https://api.github.com/users/antrik/gists{/gist_id}","starred_url":"https://api.github.com/users/antrik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antrik/subscriptions","organizations_url":"https://api.github.com/users/antrik/orgs","repos_url":"https://api.github.com/users/antrik/repos","events_url":"https://api.github.com/users/antrik/events{/privacy}","received_events_url":"https://api.github.com/users/antrik/received_events","type":"User","site_admin":false},"body":"As evidenced by the newly added \"concurrent_senders\" test case, the\r\nprevious approach never actually worked with concurrent large transfers:\r\nwhen trying to \"push back\" packets to the \"receiver\" FD of the socket\r\npair, they do not show up on the \"receiver\" FD again, but rather the\r\n\"sender\" FD.\r\n\r\nThis problem can be avoided entirely by automatically creating a\r\ndedicated channel for each large transfer, so the followup fragments can\r\nall be received in order, without interleaving with fragments from other\r\nsenders.\r\n\r\nThe first fragment of each large transfer is still sent through the main\r\nchannel, along with an FD for the dedicated channel; the remaining\r\nfragments are then sent through the dedicated channel.\r\n\r\nThis approach adds a slight overhead for fragmented transfers when there\r\nis no actual concurrency -- though it is way more efficient when actual\r\nconcurrency occurs than the push-back approach would have been...\r\n\r\nWith the dedicated channel, the follow-up fragments do not really need\r\nthe fragmentation headers anymore; and the header for the initial\r\nfragment could be simplified as well. However, to keep changes minimal,\r\nthis commit doesn't change the header yet -- this is left for later\r\nimprovements.","created_at":"2016-02-21T18:23:21Z","updated_at":"2016-02-21T20:19:50Z","closed_at":null,"merged_at":null,"merge_commit_sha":"83c8002f55eb0623c7c70166b9ea467c6ccfe752","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/servo/ipc-channel/pulls/38/commits","review_comments_url":"https://api.github.com/repos/servo/ipc-channel/pulls/38/comments","review_comment_url":"https://api.github.com/repos/servo/ipc-channel/pulls/comments{/number}","comments_url":"https://api.github.com/repos/servo/ipc-channel/issues/38/comments","statuses_url":"https://api.github.com/repos/servo/ipc-channel/statuses/813c62d79328d75b30f755ec2927435436e615b1","head":{"label":"antrik:fix-concurrent-send","ref":"fix-concurrent-send","sha":"813c62d79328d75b30f755ec2927435436e615b1","user":{"login":"antrik","id":19555,"avatar_url":"https://avatars.githubusercontent.com/u/19555?v=3","gravatar_id":"","url":"https://api.github.com/users/antrik","html_url":"https://github.com/antrik","followers_url":"https://api.github.com/users/antrik/followers","following_url":"https://api.github.com/users/antrik/following{/other_user}","gists_url":"https://api.github.com/users/antrik/gists{/gist_id}","starred_url":"https://api.github.com/users/antrik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antrik/subscriptions","organizations_url":"https://api.github.com/users/antrik/orgs","repos_url":"https://api.github.com/users/antrik/repos","events_url":"https://api.github.com/users/antrik/events{/privacy}","received_events_url":"https://api.github.com/users/antrik/received_events","type":"User","site_admin":false},"repo":{"id":47907924,"name":"ipc-channel","full_name":"antrik/ipc-channel","owner":{"login":"antrik","id":19555,"avatar_url":"https://avatars.githubusercontent.com/u/19555?v=3","gravatar_id":"","url":"https://api.github.com/users/antrik","html_url":"https://github.com/antrik","followers_url":"https://api.github.com/users/antrik/followers","following_url":"https://api.github.com/users/antrik/following{/other_user}","gists_url":"https://api.github.com/users/antrik/gists{/gist_id}","starred_url":"https://api.github.com/users/antrik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antrik/subscriptions","organizations_url":"https://api.github.com/users/antrik/orgs","repos_url":"https://api.github.com/users/antrik/repos","events_url":"https://api.github.com/users/antrik/events{/privacy}","received_events_url":"https://api.github.com/users/antrik/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/antrik/ipc-channel","description":"A multiprocess drop-in replacement for Rust channels","fork":true,"url":"https://api.github.com/repos/antrik/ipc-channel","forks_url":"https://api.github.com/repos/antrik/ipc-channel/forks","keys_url":"https://api.github.com/repos/antrik/ipc-channel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/antrik/ipc-channel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/antrik/ipc-channel/teams","hooks_url":"https://api.github.com/repos/antrik/ipc-channel/hooks","issue_events_url":"https://api.github.com/repos/antrik/ipc-channel/issues/events{/number}","events_url":"https://api.github.com/repos/antrik/ipc-channel/events","assignees_url":"https://api.github.com/repos/antrik/ipc-channel/assignees{/user}","branches_url":"https://api.github.com/repos/antrik/ipc-channel/branches{/branch}","tags_url":"https://api.github.com/repos/antrik/ipc-channel/tags","blobs_url":"https://api.github.com/repos/antrik/ipc-channel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/antrik/ipc-channel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/antrik/ipc-channel/git/refs{/sha}","trees_url":"https://api.github.com/repos/antrik/ipc-channel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/antrik/ipc-channel/statuses/{sha}","languages_url":"https://api.github.com/repos/antrik/ipc-channel/languages","stargazers_url":"https://api.github.com/repos/antrik/ipc-channel/stargazers","contributors_url":"https://api.github.com/repos/antrik/ipc-channel/contributors","subscribers_url":"https://api.github.com/repos/antrik/ipc-channel/subscribers","subscription_url":"https://api.github.com/repos/antrik/ipc-channel/subscription","commits_url":"https://api.github.com/repos/antrik/ipc-channel/commits{/sha}","git_commits_url":"https://api.github.com/repos/antrik/ipc-channel/git/commits{/sha}","comments_url":"https://api.github.com/repos/antrik/ipc-channel/comments{/number}","issue_comment_url":"https://api.github.com/repos/antrik/ipc-channel/issues/comments{/number}","contents_url":"https://api.github.com/repos/antrik/ipc-channel/contents/{+path}","compare_url":"https://api.github.com/repos/antrik/ipc-channel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/antrik/ipc-channel/merges","archive_url":"https://api.github.com/repos/antrik/ipc-channel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/antrik/ipc-channel/downloads","issues_url":"https://api.github.com/repos/antrik/ipc-channel/issues{/number}","pulls_url":"https://api.github.com/repos/antrik/ipc-channel/pulls{/number}","milestones_url":"https://api.github.com/repos/antrik/ipc-channel/milestones{/number}","notifications_url":"https://api.github.com/repos/antrik/ipc-channel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/antrik/ipc-channel/labels{/name}","releases_url":"https://api.github.com/repos/antrik/ipc-channel/releases{/id}","deployments_url":"https://api.github.com/repos/antrik/ipc-channel/deployments","created_at":"2015-12-13T05:51:19Z","updated_at":"2015-12-13T05:51:20Z","pushed_at":"2016-02-21T18:20:23Z","git_url":"git://github.com/antrik/ipc-channel.git","ssh_url":"git@github.com:antrik/ipc-channel.git","clone_url":"https://github.com/antrik/ipc-channel.git","svn_url":"https://github.com/antrik/ipc-channel","homepage":"","size":202,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"servo:master","ref":"master","sha":"b33b6a3e47d396227ff3fdd400f960016a951d00","user":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"repo":{"id":37943142,"name":"ipc-channel","full_name":"servo/ipc-channel","owner":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/servo/ipc-channel","description":"A multiprocess drop-in replacement for Rust channels","fork":false,"url":"https://api.github.com/repos/servo/ipc-channel","forks_url":"https://api.github.com/repos/servo/ipc-channel/forks","keys_url":"https://api.github.com/repos/servo/ipc-channel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/servo/ipc-channel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/servo/ipc-channel/teams","hooks_url":"https://api.github.com/repos/servo/ipc-channel/hooks","issue_events_url":"https://api.github.com/repos/servo/ipc-channel/issues/events{/number}","events_url":"https://api.github.com/repos/servo/ipc-channel/events","assignees_url":"https://api.github.com/repos/servo/ipc-channel/assignees{/user}","branches_url":"https://api.github.com/repos/servo/ipc-channel/branches{/branch}","tags_url":"https://api.github.com/repos/servo/ipc-channel/tags","blobs_url":"https://api.github.com/repos/servo/ipc-channel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/servo/ipc-channel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/servo/ipc-channel/git/refs{/sha}","trees_url":"https://api.github.com/repos/servo/ipc-channel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/servo/ipc-channel/statuses/{sha}","languages_url":"https://api.github.com/repos/servo/ipc-channel/languages","stargazers_url":"https://api.github.com/repos/servo/ipc-channel/stargazers","contributors_url":"https://api.github.com/repos/servo/ipc-channel/contributors","subscribers_url":"https://api.github.com/repos/servo/ipc-channel/subscribers","subscription_url":"https://api.github.com/repos/servo/ipc-channel/subscription","commits_url":"https://api.github.com/repos/servo/ipc-channel/commits{/sha}","git_commits_url":"https://api.github.com/repos/servo/ipc-channel/git/commits{/sha}","comments_url":"https://api.github.com/repos/servo/ipc-channel/comments{/number}","issue_comment_url":"https://api.github.com/repos/servo/ipc-channel/issues/comments{/number}","contents_url":"https://api.github.com/repos/servo/ipc-channel/contents/{+path}","compare_url":"https://api.github.com/repos/servo/ipc-channel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/servo/ipc-channel/merges","archive_url":"https://api.github.com/repos/servo/ipc-channel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/servo/ipc-channel/downloads","issues_url":"https://api.github.com/repos/servo/ipc-channel/issues{/number}","pulls_url":"https://api.github.com/repos/servo/ipc-channel/pulls{/number}","milestones_url":"https://api.github.com/repos/servo/ipc-channel/milestones{/number}","notifications_url":"https://api.github.com/repos/servo/ipc-channel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/servo/ipc-channel/labels{/name}","releases_url":"https://api.github.com/repos/servo/ipc-channel/releases{/id}","deployments_url":"https://api.github.com/repos/servo/ipc-channel/deployments","created_at":"2015-06-23T20:28:18Z","updated_at":"2016-02-04T20:13:29Z","pushed_at":"2016-02-21T18:23:21Z","git_url":"git://github.com/servo/ipc-channel.git","ssh_url":"git@github.com:servo/ipc-channel.git","clone_url":"https://github.com/servo/ipc-channel.git","svn_url":"https://github.com/servo/ipc-channel","homepage":"","size":197,"stargazers_count":83,"watchers_count":83,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":12,"mirror_url":null,"open_issues_count":9,"forks":12,"open_issues":9,"watchers":83,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/38"},"html":{"href":"https://github.com/servo/ipc-channel/pull/38"},"issue":{"href":"https://api.github.com/repos/servo/ipc-channel/issues/38"},"comments":{"href":"https://api.github.com/repos/servo/ipc-channel/issues/38/comments"},"review_comments":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/38/comments"},"review_comment":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/servo/ipc-channel/pulls/38/commits"},"statuses":{"href":"https://api.github.com/repos/servo/ipc-channel/statuses/813c62d79328d75b30f755ec2927435436e615b1"}}}}