Skip to content
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

Support calls to the SunOS Doors API #1404

Merged
merged 2 commits into from
Jun 25, 2019

Conversation

robertdfrench
Copy link
Contributor

Doors are a lightweight IPC mechanism available in libc on Solaris & illumos. They are like unix domain sockets, but faster and more pleasant to work with.

Marking this as a draft until I have included the full api.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 23, 2019

@bgermann could you review this?

@robertdfrench
Copy link
Contributor Author

Might be prudent to add illumos CI (#1405) before this goes forward. I have some demo client code to prove that this works (https://github.com/robertdfrench/rusty-doors/tree/move-unsafe-to-libc/examples/narf), and can add more for the remaining door api routines, but I'm not sure how to include that in a meaningful way without CI.

@bgermann
Copy link
Contributor

bgermann commented Jun 24, 2019

This should be okay for each supported solarish platform (10, 11, illumos) up to now. I do not see any problem with merging this.

@robertdfrench robertdfrench marked this pull request as ready for review June 24, 2019 16:13
@robertdfrench
Copy link
Contributor Author

Sounds good to me! There are more doors api calls still to be implemented, but these three are enough to make doors usable in Rust. If this is merged as is, I could focus on the the remainder of the API after we reach a CI strategy via #1405.

@jasonbking
Copy link
Contributor

This all looks good. One thing that would probably be good to document somewhere (though I don't know where the appropriate place would be) how to safely use rust in a door server (acting as a doors client should be fine). When door_create is called, a thread pool is created and as requests are received they are dispatched to the function passed into the door_create function. At the end of the request, the service thread has to call door_return to return the results to the client.

Unfortunately, one of the side effects of calling door_return (which must be called to complete the client request), is that it unceremoniously resets the stack pointer -- the moral equivalent of a longjmp back to the door dispatch function (as far as the door server is concerned -- more happens under the covers to deliver the return data to the calling thread). This means that all the drop methods that would normally be invoked at a return statement will not be invoked at a door_return call. That is probably worth mentioning.

I'm not sure there's a way to wrap the door_return call in a way that could insulate any rust door servers from having to worry about that -- though maybe someone more familiar might have some suggestions?

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 25, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 25, 2019

📌 Commit ba459b7 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Jun 25, 2019

⌛ Testing commit ba459b7 with merge 40c73a6...

bors added a commit that referenced this pull request Jun 25, 2019
Support calls to the SunOS Doors API

Doors are a lightweight IPC mechanism available in libc on Solaris & illumos. They are like unix domain sockets, but faster and more pleasant to work with.

* Brief introduction: ["Doors" in SolarisTM: Lightweight RPC using File Descriptors](http://www.kohala.com/start/papers.others/doors.html)
* Relevant manual pages: [DOOR_CALL(3C)](https://illumos.org/man/3C/door_call), [DOOR_CREATE(3C)](https://illumos.org/man/3C/door_create)
* Tutorial I wrote: ["Revolving Doors": A tutorial on the Illumos Doors API](https://github.com/robertdfrench/revolving-door)

Marking this as a draft until I have included the full api.
@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 25, 2019

Unfortunately, one of the side effects of calling door_return (which must be called to complete the client request), is that it unceremoniously resets the stack pointer -- the moral equivalent of a longjmp back to the door dispatch function (as far as the door server is concerned -- more happens under the covers to deliver the return data to the calling thread).

Can you check if door_create has the returns_twice attribute in LLVM (e.g. in the clang headers?). We might need to add this attribute here, e.g., using #[cfg_attr(feature = "unstable", c_ffi_returns_twice)], to indicate to LLVM that door_create is similar to setjmp.

This means that all the drop methods that would normally be invoked at a return statement will not be invoked at a door_return call. That is probably worth mentioning.

This is probably only safe as long as none of the types that live in the stack frames that get deallocated by calling door_return has destructors. E.g. if you were to replace door_create with catch_unwind and drop_return with panic!, this is only safe if panicking invokes no destructors until after the panic is caught by the catch_unwind.

@jasonbking
Copy link
Contributor

It doesn't return twice -- it's more like longjmp than setjmp. Note door_return doesn't use either mechanism, only that the behavior in terms of the stack and registers is similar to longjmp (and on illumos longjmp doesn't perform any stack unwinding, it just loads the new register values & jumps).

A long sought after feature would be to have some sort of two-phase return from a door server thread -- one which returns the values to the caller, and then another component that'd allow the server thread to cleanup after the return data has been sent to the caller, but that hasn't happened yet (and it's unclear how difficult that would be to implement).

Would maybe a 'noreturn' attribute be better (though the clang/LLVM docs are a bit vague on what it actually does/expects such functions to do).

@bors
Copy link
Contributor

bors commented Jun 25, 2019

☀️ Test successful - checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 40c73a6 to master...

@bors bors merged commit ba459b7 into rust-lang:master Jun 25, 2019
@robertdfrench
Copy link
Contributor Author

@jasonbking thank you for pointing this out, I had not understood that fully. I will submit a new PR to document this risk (and maybe gather feedback about where that documentation should live). As for using it safely, the work here is extracted from a crate that I am making that would (hopefully) allow users to write door server procedures in a way that smells syntactically like Rust's closures:

https://github.com/robertdfrench/rusty-doors/blob/caab2f4ef9651349a54b32d47766530949a52b61/examples/narf/src/main.rs#L20-L22

I do this by implementing a ServerProcedure trait that exposes a doors-compatible server procedure named "C" which wraps a user-supplied rust function named "rust":

https://github.com/robertdfrench/rusty-doors/blob/caab2f4ef9651349a54b32d47766530949a52b61/src/lib.rs#L47-L68

My specific implementation is certainly dubious, but I think in general it sounds like this strategy would allow for drop to work as normal, because after the inner "rust" function returns, the outer "C" function only has an array of c_char and an array of door descriptors, neither of which have denstructors. Or, am I wrong in thinking that those arrays will be cleaned up by the kernel at some point? I think you told me this before and maybe it didn't sink in right -- whose responsibility is it to free the server procedure arguments?

@jasonbking
Copy link
Contributor

The parameters into the function I think should be fine (though I'd need to confirm), though if you want to return any data, that will need to survive beyond the 'rust' function to be passed to the door_return function. Common techniques used are to store the data on the stack -- as long as what's there in rust doesn't have a drop method, that should be ok, or for larger amounts of data, a pthread_key is used w/ a cleanup function registered -- that way if the code that manages the door server thread pool kills the thread, the cleanup function gets run to release the memory -- not ideal since it means the memory could potentially be sticking around a lot longer than necessary, but it works.

Of course if you just pass back file descriptors, that's not a problem. It's just when returning a block of memory that things can get tricky.

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.

6 participants