Skip to content

Lots of changes, not sure if actually safe yet. #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 7, 2018
Merged

Lots of changes, not sure if actually safe yet. #68

merged 5 commits into from
Feb 7, 2018

Conversation

kyren
Copy link
Contributor

@kyren kyren commented Feb 7, 2018

  • Make Lua Send
  • Add Send bounds to (nearly) all instances where userdata and functions are
    passed to Lua
  • Add a "scope" method which takes a callback that accepts a Scope, and give
    Scope the ability to create functions and userdata that are !Send, and also
    functions that are not 'static!.

* Make Lua Send
* Add Send bounds to (nearly) all instances where userdata and functions are
  passed to Lua
* Add a "scope" method which takes a callback that accepts a `Scope`, and give
  `Scope` the ability to create functions and userdata that are !Send, *and also
  functions that are not even 'static!*.
@kyren
Copy link
Contributor Author

kyren commented Feb 7, 2018

This is a big change, and I really feel like I might be missing something re: safety, so it'd be great if anybody wants to take a look at it and tell me how I've inevitably missed some crucial safety aspect.

If this is a viable strategy, though, it fixes #40 and.. more or less #56 as well.

@kyren
Copy link
Contributor Author

kyren commented Feb 7, 2018

A key realization for me was that everywhere that I've ever NEEDED to give a !Send type to Lua, it has always been some sort of lock, like RwLockReadGuard etc. These also always need some sort of mechanism to manually expire them and ensure that they don't leak out, so in this way, Lua::scope does double duty by making it safe for Lua to be Send, as well as making the expiration mechanism built in. It doesn't seem like a feature, but it is.

@kyren
Copy link
Contributor Author

kyren commented Feb 7, 2018

One thing that's not tested here that should be tested is accessing expired values through rlua::Function and rlua::AnyUserData. I believe the behavior will be that if Function is called, it will just always error, and AnyUserData will behave as though it is not any type at all.

@kyren
Copy link
Contributor Author

kyren commented Feb 7, 2018

I've slept on it, and discussed it within chucklefish, and I still feel pretty good about this change. We, at least, can't figure out a way to trick it into unsafety (famous last words).

I think I'm going to go ahead and merge this into master, but I'd still like to discuss it before doing another major rlua release.

It is potentially the most annoying API change yet because of the new requirement of userdata to be Send, I don't really have a good sense of the amount of pain this will cause.

kyren added 2 commits February 7, 2018 16:51
… internal

It is part of the contract that only LuaRef types constructed from the same
parent Lua state are passed into Lua, so generating a panic there is not an
internal error.
@kyren kyren merged commit 728e8ea into master Feb 7, 2018
@kyren
Copy link
Contributor Author

kyren commented Feb 8, 2018

Well, I merged this, but I actually screwed up the lifetimes in Lua::scope, and it's possible to reference things that don't live long enough.

I'm currently staring that the crossbeam crate trying to figure out the correct technique.

@kyren kyren deleted the scope branch October 1, 2018 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant