{"action":"created","issue":{"url":"https://api.github.com/repos/kasper/phoenix/issues/70","labels_url":"https://api.github.com/repos/kasper/phoenix/issues/70/labels{/name}","comments_url":"https://api.github.com/repos/kasper/phoenix/issues/70/comments","events_url":"https://api.github.com/repos/kasper/phoenix/issues/70/events","html_url":"https://github.com/kasper/phoenix/issues/70","id":128423935,"number":70,"title":"Javascript exceptions in keyhandler functions are not caught","user":{"login":"pascalw","id":304603,"avatar_url":"https://avatars.githubusercontent.com/u/304603?v=3","gravatar_id":"","url":"https://api.github.com/users/pascalw","html_url":"https://github.com/pascalw","followers_url":"https://api.github.com/users/pascalw/followers","following_url":"https://api.github.com/users/pascalw/following{/other_user}","gists_url":"https://api.github.com/users/pascalw/gists{/gist_id}","starred_url":"https://api.github.com/users/pascalw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pascalw/subscriptions","organizations_url":"https://api.github.com/users/pascalw/orgs","repos_url":"https://api.github.com/users/pascalw/repos","events_url":"https://api.github.com/users/pascalw/events{/privacy}","received_events_url":"https://api.github.com/users/pascalw/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kasper/phoenix/labels/bug","name":"bug","color":"fc2929"}],"state":"open","locked":false,"assignee":{"login":"kasper","id":925851,"avatar_url":"https://avatars.githubusercontent.com/u/925851?v=3","gravatar_id":"","url":"https://api.github.com/users/kasper","html_url":"https://github.com/kasper","followers_url":"https://api.github.com/users/kasper/followers","following_url":"https://api.github.com/users/kasper/following{/other_user}","gists_url":"https://api.github.com/users/kasper/gists{/gist_id}","starred_url":"https://api.github.com/users/kasper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kasper/subscriptions","organizations_url":"https://api.github.com/users/kasper/orgs","repos_url":"https://api.github.com/users/kasper/repos","events_url":"https://api.github.com/users/kasper/events{/privacy}","received_events_url":"https://api.github.com/users/kasper/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/kasper/phoenix/milestones/5","html_url":"https://github.com/kasper/phoenix/milestones/2.1","labels_url":"https://api.github.com/repos/kasper/phoenix/milestones/5/labels","id":1457254,"number":5,"title":"2.1","description":"","creator":{"login":"kasper","id":925851,"avatar_url":"https://avatars.githubusercontent.com/u/925851?v=3","gravatar_id":"","url":"https://api.github.com/users/kasper","html_url":"https://github.com/kasper","followers_url":"https://api.github.com/users/kasper/followers","following_url":"https://api.github.com/users/kasper/following{/other_user}","gists_url":"https://api.github.com/users/kasper/gists{/gist_id}","starred_url":"https://api.github.com/users/kasper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kasper/subscriptions","organizations_url":"https://api.github.com/users/kasper/orgs","repos_url":"https://api.github.com/users/kasper/repos","events_url":"https://api.github.com/users/kasper/events{/privacy}","received_events_url":"https://api.github.com/users/kasper/received_events","type":"User","site_admin":false},"open_issues":4,"closed_issues":1,"state":"open","created_at":"2015-12-12T13:53:59Z","updated_at":"2016-01-24T21:44:22Z","due_on":null,"closed_at":null},"comments":5,"created_at":"2016-01-24T20:44:35Z","updated_at":"2016-01-25T20:12:53Z","closed_at":null,"body":"Consider the following snippet:\r\n\r\n```js\r\nvar handler = Phoenix.bind('down', ctrlAltCmd, function() {\r\n  foo.bar(); // foo is not defined\r\n});\r\n```\r\n\r\nThis does not trigger any exception handling, so you won't notice anything went wrong.\r\n\r\nTaking a quick peek at the code I noticed that [here](https://github.com/kasper/phoenix/blob/bd2ad67b284299828d0fd24ff4f8f8e115ac8b7e/Phoenix/PHHandler.m#L28) a new context is created to execute the key handlers function. This new context has no exceptionHandler set.\r\n\r\nIs there any specific reason a new context is created here? Executing the function in the parent context or sharing the exception handler would fix this issue. Please let me know which option you think is best and I'll create a PR.\r\n"},"comment":{"url":"https://api.github.com/repos/kasper/phoenix/issues/comments/174646442","html_url":"https://github.com/kasper/phoenix/issues/70#issuecomment-174646442","issue_url":"https://api.github.com/repos/kasper/phoenix/issues/70","id":174646442,"user":{"login":"kasper","id":925851,"avatar_url":"https://avatars.githubusercontent.com/u/925851?v=3","gravatar_id":"","url":"https://api.github.com/users/kasper","html_url":"https://github.com/kasper","followers_url":"https://api.github.com/users/kasper/followers","following_url":"https://api.github.com/users/kasper/following{/other_user}","gists_url":"https://api.github.com/users/kasper/gists{/gist_id}","starred_url":"https://api.github.com/users/kasper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kasper/subscriptions","organizations_url":"https://api.github.com/users/kasper/orgs","repos_url":"https://api.github.com/users/kasper/repos","events_url":"https://api.github.com/users/kasper/events{/privacy}","received_events_url":"https://api.github.com/users/kasper/received_events","type":"User","site_admin":false},"created_at":"2016-01-25T20:12:53Z","updated_at":"2016-01-25T20:12:53Z","body":"@pascalw Thanks for taking such care in exploring this! You are right that most likely creating extensive amounts of JSContexts is expensive. Obviously, in real life you never invoke the handlers consecutively so many times that you would notice this before the memory is already deallocated. Maybe some caching could be useful. Clearly there is still an issue, if some memory is retained.\r\n\r\nUnfortunately, the JavaScriptCore documentation and headers are very obscure — especially related to memory management. This is what I’ve figured out: JavaScriptCore is still garbage collected even inside an ARC environment. This is why you need to use managed JSValues to ensure memory is released when needed. From my understanding, the allocations created within the context of the function callback are retained until the entire function is released. Obviously, this only happens when the handler is released and in Phoenix this happens rarely — basically only on context reloads. This is why technically they are not leaks and are not caught by Instruments as such. Clearly this is not desired and the allocations should be released when the callback is completed. I experimented with this a lot and the only way I found out to release the scope was to create a new JSContext for it. This way all allocations that are not stored outside of the function’s scope get released after the function is completed. I even contacted Apple technical developer support to get a comment about this, but they were not helpful at all. Maybe contacting the WebKit community could be useful as JavaScriptCore is part of the open-source project. I also spent some time earlier skimming through its source code to try to get an idea why the memory management behaves this way. I could’t find anything useful.\r\n\r\nIf you have time to debug this more or have any additional ideas, I’m happy to investigate them. We can definitely also have a short Google Hangout about this topic later this week or next week, if you feel it could be helpful."}}