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

&'a Headers does not allow reading Headers #47

Closed
reem opened this issue Sep 19, 2014 · 3 comments · Fixed by #48
Closed

&'a Headers does not allow reading Headers #47

reem opened this issue Sep 19, 2014 · 3 comments · Fixed by #48

Comments

@reem
Copy link
Contributor

reem commented Sep 19, 2014

I've got a branch implementing a HeadersView that solves this problem, but right now that means you can't read the Headers of anything Streaming.

@seanmonstar
Copy link
Member

Oh right, because it needs to be mutable internally. Maybe Headers could
keep a RefCell of the HashMap. It's mutability is just an implementation
detail, and shouldn't concern users.

That would also improve fighting the borrowck in matches.

@reem
Copy link
Contributor Author

reem commented Sep 19, 2014

RefCell would not be a good solution here as it would be a leaky abstraction and lead to runtime failures when code tries to get two header references with overlapping lifetimes - this code would fail, for instance:

let length = req.headers.get_ref::<ContentLength>();
let encoding = req.headers.get_ref::<TransferEncoding>();

Right now this is a compile-time failure, using RefCell would make it a runtime failure.

We would also have to return cell::Ref and cell::RefMut instead of & and &mut.

reem added a commit to reem/hyper that referenced this issue Sep 20, 2014
This removes the need to receive `&mut self` in `get_ref` and `get.`

Since the interior mutability of the RWLock is needed only once,
it is safe to change the lifetime of the pointer given by read locks
as by then all mutation has been done.

Since `set` still requires `&mut self` there is no way to use the interior
mutability of the RWLock to modify an existing `&`-reference. However,
the use of interior mutability in `get_ref` means that `get_raw` is now
actually an unsafe operation because the (now `*const`) pointer could be
invalidated by a subsequent call to `get_ref.`

Fixes hyperium#47
@reem
Copy link
Contributor Author

reem commented Sep 21, 2014

Basically scratch the above, I did crazy things with RWLock in #48 that fixes this and more.

reem added a commit to reem/hyper that referenced this issue Sep 26, 2014
This removes the need to receive `&mut self` in `get_ref` and `get.`

Since the interior mutability of the RWLock is needed only once,
it is safe to change the lifetime of the pointer given by read locks
as by then all mutation has been done.

Since `set` still requires `&mut self` there is no way to use the interior
mutability of the RWLock to modify an existing `&`-reference. However,
the use of interior mutability in `get_ref` means that `get_raw` is now
actually an unsafe operation because the (now `*const`) pointer could be
invalidated by a subsequent call to `get_ref.`

Fixes hyperium#47
reem added a commit to reem/hyper that referenced this issue Sep 27, 2014
This removes the need to receive `&mut self` in `get_ref` and `get.`

Since the interior mutability of the RWLock is needed only once,
it is safe to change the lifetime of the pointer given by read locks
as by then all mutation has been done.

Since `set` still requires `&mut self` there is no way to use the interior
mutability of the RWLock to modify an existing `&`-reference. However,
the use of interior mutability in `get_ref` means that `get_raw` is now
actually an unsafe operation because the (now `*const`) pointer could be
invalidated by a subsequent call to `get_ref.`

Fixes hyperium#47
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 a pull request may close this issue.

2 participants