{"action":"created","issue":{"url":"https://api.github.com/repos/rust-lang/rust/issues/48251","repository_url":"https://api.github.com/repos/rust-lang/rust","labels_url":"https://api.github.com/repos/rust-lang/rust/issues/48251/labels{/name}","comments_url":"https://api.github.com/repos/rust-lang/rust/issues/48251/comments","events_url":"https://api.github.com/repos/rust-lang/rust/issues/48251/events","html_url":"https://github.com/rust-lang/rust/issues/48251","id":297647904,"number":48251,"title":"Regression in rlua crate tests between rustc 1.23.0 and 1.24.0, only on windows.","user":{"login":"kyren","id":1538607,"avatar_url":"https://avatars1.githubusercontent.com/u/1538607?v=4","gravatar_id":"","url":"https://api.github.com/users/kyren","html_url":"https://github.com/kyren","followers_url":"https://api.github.com/users/kyren/followers","following_url":"https://api.github.com/users/kyren/following{/other_user}","gists_url":"https://api.github.com/users/kyren/gists{/gist_id}","starred_url":"https://api.github.com/users/kyren/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kyren/subscriptions","organizations_url":"https://api.github.com/users/kyren/orgs","repos_url":"https://api.github.com/users/kyren/repos","events_url":"https://api.github.com/users/kyren/events{/privacy}","received_events_url":"https://api.github.com/users/kyren/received_events","type":"User","site_admin":false},"labels":[{"id":650731663,"url":"https://api.github.com/repos/rust-lang/rust/labels/C-bug","name":"C-bug","color":"f5f1fd","default":false},{"id":123109,"url":"https://api.github.com/repos/rust-lang/rust/labels/O-windows","name":"O-windows","color":"6e6ec0","default":false},{"id":262252840,"url":"https://api.github.com/repos/rust-lang/rust/labels/regression-from-stable-to-stable","name":"regression-from-stable-to-stable","color":"e4008a","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-02-16T00:43:37Z","updated_at":"2018-02-18T15:17:54Z","closed_at":null,"author_association":"NONE","body":"[The following explanation of the problem contains a lot of assumptions about the problem's nature, I could be extremely wrong!  Take this explanation with a grain of salt]\r\n\r\nWild guess, it has something to do with https://github.com/rust-lang/rust/pull/46833.\r\n\r\nSo, I'm not sure this is a rust bug *exactly*, but I wanted to make you aware of a crate regression that's causing me quite a lot of problems.  Ultimately, I believe this comes from me doing some things that are not.. exactly *definitely* kosher, but that I don't have a convenient workaround for.\r\n\r\nUltimately, the problem stems from the Lua C API having error handling *fundamentally* based around setjmp / longjmp.  In order to write a usable wrapper in Rust, without having to write inconvenient, slow C shims, Rust must, at times, call into Lua C API functions that in turn trigger a longjmp, and that longjmp must necessarily pass over Rust frames.\r\n\r\nI realize how unsafe this is!  However, there are some limitations here that I think make it at least somewhat reasonable to be able to do:\r\n\r\n1) There only Copy types on the rust stack frame being jumped over.\r\n2) Rust is never calling setjmp, only triggering a longjmp through a Lua C API call (such as lua_error).\r\n3) Making this unsafe to do means that it is impossible to wrap a C API such as Lua's without resorting to writing more C.\r\n\r\nThis last point is unfortunate, and causes me a lot of headaches, and at least one strange practical problem.  We at Chucklefish build rlua for consoles, but for consoles the gcc crate does not work, and we have to build Lua out of tree (also to include Lua console specific patches).  Needing to include C shims in rlua means that we would also have to copy paste those shims into the console projects, and this is a less than ideal situation.\r\n\r\nThe reason I think this issue has to do with https://github.com/rust-lang/rust/pull/46833 is that I can actually fix the regression if I simply add an `#[unwind]` attribute to each rust function that calls lua_error.  However, this is only necessary for some reason *on windows*, and the attribute is currently unstable!\r\n\r\nI think it's completely reasonable to need to mark functions as `#[unwind]` if they will trigger a longjmp, but I'm not exactly sure what to do in the meantime until `unwind_attributes` becomes stable.  Maybe this behavior on windows IS simply a bug?  Maybe it's a bug on linux / macos that this doesn't abort?  I would especially not like to have to write C shims if ultimately I can get rid of them when `unwind_attributes` becomes stable.\r\n\r\n(Side Note: Chucklefish actually uses the stable rust compiler in production now, so this is especially annoying.  We were very excited about the 1.24 release, because it includes incremental compilation and rustfmt-preview while also being stable, and we noticed this bug in trying to switch to the stable compiler on windows.)\r\n\r\nEdit: If people have to dig into the `rlua` source, *I'm sorry*, it is a tangled nest of \"unsafe, unsafe everywhere\".  The best example to look at is the `test_error` test, and the method `Lua::create_callback_function`.  Adding the `#[unwind]` attribute to: `callback_call_impl`, `safe_pcall`, and `safe_xpcall` fixes that test on windows, on unstable.\r\n\r\nEdit 2: Edited for clarity, but also I need to point out that I need to do *something*, because this bug currently makes `rlua` just basically broken on windows.  It's not \"just\" a test failure, error handling in general will just abort."},"comment":{"url":"https://api.github.com/repos/rust-lang/rust/issues/comments/366523173","html_url":"https://github.com/rust-lang/rust/issues/48251#issuecomment-366523173","issue_url":"https://api.github.com/repos/rust-lang/rust/issues/48251","id":366523173,"user":{"login":"SoniEx2","id":1779619,"avatar_url":"https://avatars2.githubusercontent.com/u/1779619?v=4","gravatar_id":"","url":"https://api.github.com/users/SoniEx2","html_url":"https://github.com/SoniEx2","followers_url":"https://api.github.com/users/SoniEx2/followers","following_url":"https://api.github.com/users/SoniEx2/following{/other_user}","gists_url":"https://api.github.com/users/SoniEx2/gists{/gist_id}","starred_url":"https://api.github.com/users/SoniEx2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SoniEx2/subscriptions","organizations_url":"https://api.github.com/users/SoniEx2/orgs","repos_url":"https://api.github.com/users/SoniEx2/repos","events_url":"https://api.github.com/users/SoniEx2/events{/privacy}","received_events_url":"https://api.github.com/users/SoniEx2/received_events","type":"User","site_admin":false},"created_at":"2018-02-18T15:17:54Z","updated_at":"2018-02-18T15:17:54Z","author_association":"NONE","body":"I guess when they say it's a \"C API\" they are not kidding. :p\r\n\r\nWould it make sense to make an standalone wrapper, called \"Universal API for Lua, with C calling conventions\"?"}}
{"actor":{"display_login":"SoniEx2"}}