{"action":"created","comment":{"url":"https://api.github.com/repos/pybind/pybind11/pulls/comments/223476593","pull_request_review_id":162613008,"id":223476593,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIyMzQ3NjU5Mw==","diff_hunk":"@@ -105,3 +105,21 @@ def test_function_signatures(doc):\n \n def test_movable_object():\n     assert m.callback_with_movable(lambda _: None) is True\n+\n+\n+def test_no_deadlock():\n+    # This code would deadlock - see issue 1525","path":"tests/test_callbacks.py","position":7,"original_position":7,"commit_id":"34e3b5932c2c98f327b274c3cb83e954e6eb94a4","original_commit_id":"34e3b5932c2c98f327b274c3cb83e954e6eb94a4","user":{"login":"davidhewitt","id":1939362,"node_id":"MDQ6VXNlcjE5MzkzNjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1939362?v=4","gravatar_id":"","url":"https://api.github.com/users/davidhewitt","html_url":"https://github.com/davidhewitt","followers_url":"https://api.github.com/users/davidhewitt/followers","following_url":"https://api.github.com/users/davidhewitt/following{/other_user}","gists_url":"https://api.github.com/users/davidhewitt/gists{/gist_id}","starred_url":"https://api.github.com/users/davidhewitt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidhewitt/subscriptions","organizations_url":"https://api.github.com/users/davidhewitt/orgs","repos_url":"https://api.github.com/users/davidhewitt/repos","events_url":"https://api.github.com/users/davidhewitt/events{/privacy}","received_events_url":"https://api.github.com/users/davidhewitt/received_events","type":"User","site_admin":false},"body":"Looks like at least on CI the deadlock is handled cleanly!","created_at":"2018-10-08T19:42:09Z","updated_at":"2018-10-08T19:42:09Z","html_url":"https://github.com/pybind/pybind11/pull/1556#discussion_r223476593","pull_request_url":"https://api.github.com/repos/pybind/pybind11/pulls/1556","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/comments/223476593"},"html":{"href":"https://github.com/pybind/pybind11/pull/1556#discussion_r223476593"},"pull_request":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/1556"}},"in_reply_to_id":223180394},"pull_request":{"url":"https://api.github.com/repos/pybind/pybind11/pulls/1556","id":220882253,"node_id":"MDExOlB1bGxSZXF1ZXN0MjIwODgyMjUz","html_url":"https://github.com/pybind/pybind11/pull/1556","diff_url":"https://github.com/pybind/pybind11/pull/1556.diff","patch_url":"https://github.com/pybind/pybind11/pull/1556.patch","issue_url":"https://api.github.com/repos/pybind/pybind11/issues/1556","number":1556,"state":"open","locked":false,"title":"Resolve threading issue with gil_scoped_acquire","user":{"login":"davidhewitt","id":1939362,"node_id":"MDQ6VXNlcjE5MzkzNjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1939362?v=4","gravatar_id":"","url":"https://api.github.com/users/davidhewitt","html_url":"https://github.com/davidhewitt","followers_url":"https://api.github.com/users/davidhewitt/followers","following_url":"https://api.github.com/users/davidhewitt/following{/other_user}","gists_url":"https://api.github.com/users/davidhewitt/gists{/gist_id}","starred_url":"https://api.github.com/users/davidhewitt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidhewitt/subscriptions","organizations_url":"https://api.github.com/users/davidhewitt/orgs","repos_url":"https://api.github.com/users/davidhewitt/repos","events_url":"https://api.github.com/users/davidhewitt/events{/privacy}","received_events_url":"https://api.github.com/users/davidhewitt/received_events","type":"User","site_admin":false},"body":"It was possible for python threads to execute c++ code which would subsequently call gil_scoped_acquire, incorrectly conclude that the gil was not held, and then deadlock.\r\n\r\nThe most basic example I can build of the failing code is:\r\n\r\n```c++\r\n#include <pybind11/pybind11.h>\r\n#include <pybind11/functional.h>\r\n\r\nPYBIND11_MODULE(pybind11test, m)\r\n{\r\n  m.def(\"deadlock\", [] (std::function<void ()> f) { f(); });\r\n}\r\n```\r\nand then invoked in python as so:\r\n\r\n```python\r\nimport pybind11test\r\n\r\nimport threading\r\n\r\nthread1 = threading.Thread(target=pybind11test.deadlock, args=(lambda: None,))\r\nthread1.start()\r\n\r\nthread2 = threading.Thread(target=pybind11test.deadlock, args=(lambda: None,))\r\nthread2.start() \r\n\r\nthread1.join()\r\nthread2.join()\r\n```\r\n\r\nI believe that this patch will resolve both #1525 and #1308.\r\n\r\nTo me this is a critical problem, as I often work with python threads. I would be very grateful if this can be reviewed and merged in time for the pybind11 2.3 release.\r\n\r\nI am happy to make any changes you feel necessary, including writing a different solution altogether if you have a better one in mind.\r\n\r\nThanks in advance,\r\nDavid","created_at":"2018-10-06T11:57:15Z","updated_at":"2018-10-08T19:42:09Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f390cb997f80e3615d29e960ee658502668331c0","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/pybind/pybind11/pulls/1556/commits","review_comments_url":"https://api.github.com/repos/pybind/pybind11/pulls/1556/comments","review_comment_url":"https://api.github.com/repos/pybind/pybind11/pulls/comments{/number}","comments_url":"https://api.github.com/repos/pybind/pybind11/issues/1556/comments","statuses_url":"https://api.github.com/repos/pybind/pybind11/statuses/34e3b5932c2c98f327b274c3cb83e954e6eb94a4","head":{"label":"davidhewitt:issue_1525","ref":"issue_1525","sha":"34e3b5932c2c98f327b274c3cb83e954e6eb94a4","user":{"login":"davidhewitt","id":1939362,"node_id":"MDQ6VXNlcjE5MzkzNjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1939362?v=4","gravatar_id":"","url":"https://api.github.com/users/davidhewitt","html_url":"https://github.com/davidhewitt","followers_url":"https://api.github.com/users/davidhewitt/followers","following_url":"https://api.github.com/users/davidhewitt/following{/other_user}","gists_url":"https://api.github.com/users/davidhewitt/gists{/gist_id}","starred_url":"https://api.github.com/users/davidhewitt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidhewitt/subscriptions","organizations_url":"https://api.github.com/users/davidhewitt/orgs","repos_url":"https://api.github.com/users/davidhewitt/repos","events_url":"https://api.github.com/users/davidhewitt/events{/privacy}","received_events_url":"https://api.github.com/users/davidhewitt/received_events","type":"User","site_admin":false},"repo":{"id":134954975,"node_id":"MDEwOlJlcG9zaXRvcnkxMzQ5NTQ5NzU=","name":"pybind11","full_name":"davidhewitt/pybind11","private":false,"owner":{"login":"davidhewitt","id":1939362,"node_id":"MDQ6VXNlcjE5MzkzNjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1939362?v=4","gravatar_id":"","url":"https://api.github.com/users/davidhewitt","html_url":"https://github.com/davidhewitt","followers_url":"https://api.github.com/users/davidhewitt/followers","following_url":"https://api.github.com/users/davidhewitt/following{/other_user}","gists_url":"https://api.github.com/users/davidhewitt/gists{/gist_id}","starred_url":"https://api.github.com/users/davidhewitt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidhewitt/subscriptions","organizations_url":"https://api.github.com/users/davidhewitt/orgs","repos_url":"https://api.github.com/users/davidhewitt/repos","events_url":"https://api.github.com/users/davidhewitt/events{/privacy}","received_events_url":"https://api.github.com/users/davidhewitt/received_events","type":"User","site_admin":false},"html_url":"https://github.com/davidhewitt/pybind11","description":"Seamless operability between C++11 and Python","fork":true,"url":"https://api.github.com/repos/davidhewitt/pybind11","forks_url":"https://api.github.com/repos/davidhewitt/pybind11/forks","keys_url":"https://api.github.com/repos/davidhewitt/pybind11/keys{/key_id}","collaborators_url":"https://api.github.com/repos/davidhewitt/pybind11/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/davidhewitt/pybind11/teams","hooks_url":"https://api.github.com/repos/davidhewitt/pybind11/hooks","issue_events_url":"https://api.github.com/repos/davidhewitt/pybind11/issues/events{/number}","events_url":"https://api.github.com/repos/davidhewitt/pybind11/events","assignees_url":"https://api.github.com/repos/davidhewitt/pybind11/assignees{/user}","branches_url":"https://api.github.com/repos/davidhewitt/pybind11/branches{/branch}","tags_url":"https://api.github.com/repos/davidhewitt/pybind11/tags","blobs_url":"https://api.github.com/repos/davidhewitt/pybind11/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/davidhewitt/pybind11/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/davidhewitt/pybind11/git/refs{/sha}","trees_url":"https://api.github.com/repos/davidhewitt/pybind11/git/trees{/sha}","statuses_url":"https://api.github.com/repos/davidhewitt/pybind11/statuses/{sha}","languages_url":"https://api.github.com/repos/davidhewitt/pybind11/languages","stargazers_url":"https://api.github.com/repos/davidhewitt/pybind11/stargazers","contributors_url":"https://api.github.com/repos/davidhewitt/pybind11/contributors","subscribers_url":"https://api.github.com/repos/davidhewitt/pybind11/subscribers","subscription_url":"https://api.github.com/repos/davidhewitt/pybind11/subscription","commits_url":"https://api.github.com/repos/davidhewitt/pybind11/commits{/sha}","git_commits_url":"https://api.github.com/repos/davidhewitt/pybind11/git/commits{/sha}","comments_url":"https://api.github.com/repos/davidhewitt/pybind11/comments{/number}","issue_comment_url":"https://api.github.com/repos/davidhewitt/pybind11/issues/comments{/number}","contents_url":"https://api.github.com/repos/davidhewitt/pybind11/contents/{+path}","compare_url":"https://api.github.com/repos/davidhewitt/pybind11/compare/{base}...{head}","merges_url":"https://api.github.com/repos/davidhewitt/pybind11/merges","archive_url":"https://api.github.com/repos/davidhewitt/pybind11/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/davidhewitt/pybind11/downloads","issues_url":"https://api.github.com/repos/davidhewitt/pybind11/issues{/number}","pulls_url":"https://api.github.com/repos/davidhewitt/pybind11/pulls{/number}","milestones_url":"https://api.github.com/repos/davidhewitt/pybind11/milestones{/number}","notifications_url":"https://api.github.com/repos/davidhewitt/pybind11/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/davidhewitt/pybind11/labels{/name}","releases_url":"https://api.github.com/repos/davidhewitt/pybind11/releases{/id}","deployments_url":"https://api.github.com/repos/davidhewitt/pybind11/deployments","created_at":"2018-05-26T11:18:38Z","updated_at":"2018-05-26T11:18:41Z","pushed_at":"2018-10-06T11:51:25Z","git_url":"git://github.com/davidhewitt/pybind11.git","ssh_url":"git@github.com:davidhewitt/pybind11.git","clone_url":"https://github.com/davidhewitt/pybind11.git","svn_url":"https://github.com/davidhewitt/pybind11","homepage":"","size":3800,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"NOASSERTION","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"pybind:master","ref":"master","sha":"177713fa4ea49c5594b5568279757f22354fa000","user":{"login":"pybind","id":17565521,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE3NTY1NTIx","avatar_url":"https://avatars2.githubusercontent.com/u/17565521?v=4","gravatar_id":"","url":"https://api.github.com/users/pybind","html_url":"https://github.com/pybind","followers_url":"https://api.github.com/users/pybind/followers","following_url":"https://api.github.com/users/pybind/following{/other_user}","gists_url":"https://api.github.com/users/pybind/gists{/gist_id}","starred_url":"https://api.github.com/users/pybind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pybind/subscriptions","organizations_url":"https://api.github.com/users/pybind/orgs","repos_url":"https://api.github.com/users/pybind/repos","events_url":"https://api.github.com/users/pybind/events{/privacy}","received_events_url":"https://api.github.com/users/pybind/received_events","type":"Organization","site_admin":false},"repo":{"id":38581626,"node_id":"MDEwOlJlcG9zaXRvcnkzODU4MTYyNg==","name":"pybind11","full_name":"pybind/pybind11","private":false,"owner":{"login":"pybind","id":17565521,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE3NTY1NTIx","avatar_url":"https://avatars2.githubusercontent.com/u/17565521?v=4","gravatar_id":"","url":"https://api.github.com/users/pybind","html_url":"https://github.com/pybind","followers_url":"https://api.github.com/users/pybind/followers","following_url":"https://api.github.com/users/pybind/following{/other_user}","gists_url":"https://api.github.com/users/pybind/gists{/gist_id}","starred_url":"https://api.github.com/users/pybind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pybind/subscriptions","organizations_url":"https://api.github.com/users/pybind/orgs","repos_url":"https://api.github.com/users/pybind/repos","events_url":"https://api.github.com/users/pybind/events{/privacy}","received_events_url":"https://api.github.com/users/pybind/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/pybind/pybind11","description":"Seamless operability between C++11 and Python","fork":false,"url":"https://api.github.com/repos/pybind/pybind11","forks_url":"https://api.github.com/repos/pybind/pybind11/forks","keys_url":"https://api.github.com/repos/pybind/pybind11/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pybind/pybind11/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pybind/pybind11/teams","hooks_url":"https://api.github.com/repos/pybind/pybind11/hooks","issue_events_url":"https://api.github.com/repos/pybind/pybind11/issues/events{/number}","events_url":"https://api.github.com/repos/pybind/pybind11/events","assignees_url":"https://api.github.com/repos/pybind/pybind11/assignees{/user}","branches_url":"https://api.github.com/repos/pybind/pybind11/branches{/branch}","tags_url":"https://api.github.com/repos/pybind/pybind11/tags","blobs_url":"https://api.github.com/repos/pybind/pybind11/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pybind/pybind11/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pybind/pybind11/git/refs{/sha}","trees_url":"https://api.github.com/repos/pybind/pybind11/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pybind/pybind11/statuses/{sha}","languages_url":"https://api.github.com/repos/pybind/pybind11/languages","stargazers_url":"https://api.github.com/repos/pybind/pybind11/stargazers","contributors_url":"https://api.github.com/repos/pybind/pybind11/contributors","subscribers_url":"https://api.github.com/repos/pybind/pybind11/subscribers","subscription_url":"https://api.github.com/repos/pybind/pybind11/subscription","commits_url":"https://api.github.com/repos/pybind/pybind11/commits{/sha}","git_commits_url":"https://api.github.com/repos/pybind/pybind11/git/commits{/sha}","comments_url":"https://api.github.com/repos/pybind/pybind11/comments{/number}","issue_comment_url":"https://api.github.com/repos/pybind/pybind11/issues/comments{/number}","contents_url":"https://api.github.com/repos/pybind/pybind11/contents/{+path}","compare_url":"https://api.github.com/repos/pybind/pybind11/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pybind/pybind11/merges","archive_url":"https://api.github.com/repos/pybind/pybind11/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pybind/pybind11/downloads","issues_url":"https://api.github.com/repos/pybind/pybind11/issues{/number}","pulls_url":"https://api.github.com/repos/pybind/pybind11/pulls{/number}","milestones_url":"https://api.github.com/repos/pybind/pybind11/milestones{/number}","notifications_url":"https://api.github.com/repos/pybind/pybind11/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pybind/pybind11/labels{/name}","releases_url":"https://api.github.com/repos/pybind/pybind11/releases{/id}","deployments_url":"https://api.github.com/repos/pybind/pybind11/deployments","created_at":"2015-07-05T19:46:48Z","updated_at":"2018-10-08T18:58:11Z","pushed_at":"2018-10-08T08:23:35Z","git_url":"git://github.com/pybind/pybind11.git","ssh_url":"git@github.com:pybind/pybind11.git","clone_url":"https://github.com/pybind/pybind11.git","svn_url":"https://github.com/pybind/pybind11","homepage":"","size":3809,"stargazers_count":4068,"watchers_count":4068,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":527,"mirror_url":null,"archived":false,"open_issues_count":285,"license":{"key":"other","name":"NOASSERTION","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":527,"open_issues":285,"watchers":4068,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/1556"},"html":{"href":"https://github.com/pybind/pybind11/pull/1556"},"issue":{"href":"https://api.github.com/repos/pybind/pybind11/issues/1556"},"comments":{"href":"https://api.github.com/repos/pybind/pybind11/issues/1556/comments"},"review_comments":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/1556/comments"},"review_comment":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/pybind/pybind11/pulls/1556/commits"},"statuses":{"href":"https://api.github.com/repos/pybind/pybind11/statuses/34e3b5932c2c98f327b274c3cb83e954e6eb94a4"}},"author_association":"NONE"}}
{"actor":{"display_login":"davidhewitt"}}