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

Added method to unpoison poisoned RWARCs #7392

Closed
wants to merge 1 commit into from

Conversation

mstewartgallus
Copy link
Contributor

This is mostly a prototype for future changes. I assume more
complicated methods like unpoison_with_condvar may need to be added
but the whole complicated mess of varying types of writes, and reads
need to be refactored, and turned into a cleaner, and better interface
anyways. Personally, I need this operation to implement functions for
the library of concurrent data structures I am working on but this
operation may be of interest to others.

A rustic style for cleaning up the interface is unclear to me at the
moment but a possible example in Haskell might be to use a monad, and
have write simply be a consequence of the logically more primitive
read, and unpoison operations. This monadic approach in Haskell might
look like the following:

atomicIncrement var = atomically $ do
     x <- read var
     var $= x + 1

This is mostly a prototype for future changes. I assume more
complicated methods like unpoison_with_condvar may need to be added
but the whole complicated mess of varying types of writes, and reads
need to be refactored, and turned into a cleaner, and better interface
anyways. Personally, I need this operation to implement functions for
the library of concurrent data structures I am working on but this
operation may be of interest to others.

A rustic style for cleaning up the interface is unclear to me at the
moment but a possible example in Haskell might be to use a monad, and
have write simply be a consequence of the logically more primitive
read, and unpoison operations. This monadic approach in Haskell might
look like the following:

atomicIncrement var = atomically $ do
	  x <- read var
	  var $= x + 1
@thestinger
Copy link
Contributor

cc @bblum

@bblum
Copy link
Contributor

bblum commented Jun 26, 2013

I'm not sure having an unpoison method is the right approach here. Any task that wants to do this will presumably need to atomically make sure their unpoison attempt was successful w.r.t a subsequent access to the RWARC. I have two ideas for how it should be --

When I first wrote poisoning, I thought of it as making an RWARC implicitly be sort of a back-channel for linked failure propagation. I could imagine a use case that wanted to avoid poisoning, and at the time figured the best way would actually be to offer a RWARC::new_without_poisoning() way to construct them, which simply disables the check (by stashing a bool inside the rwarc representation; a negligible performance hit). Keep in mind that, as it is, the poisoning is not actually important for interface soundness, and maybe it should even be off by default.

The thing I think this function wants to be, which could coexist with the above, is an access function that provides optional data instead of failing automatically:

fn read_unless_poisoned(blk: fn(Option<&T>) -> U) -> U
fn write_unless_poisoned(blk: fn(Option<&mut T>) -> U) -> U
fn write_cond_unless_poisoned(blk: fn(Option<(&mut T, &Condvar)>) -> U) -> U
fn write_downgrade_unless_poisoned(blk: fn(Option<RWWriteMode>) -> U) -> U

The current access functions could, of course, be implemented using those and option::unwrap.

@bblum
Copy link
Contributor

bblum commented Jun 26, 2013

Also, implementing write as a nonprimitive on top of read and unpoison would break the atomicity provided by the write-locked rwlock.

@bblum bblum closed this Jun 26, 2013
@mstewartgallus
Copy link
Contributor Author

@bblum I think having it be not higher order would be cleaner but I heard that having a thunk can be optimized better than just regular parameter passing. Having a thunk is ugly though so I'll make it just be a normal parameter. I don't think doing a micro-optimization in this case is worth it. Besides now the poison on fail stuff can be removed.

I feel silly that I missed the race condition with spawn_unsupervised.

Okay! The zeroing of destroyed data makes much more sense to me now after reading your comment on the failure check stuff mistake. I don't think I'll make that kind of mistake again.

I don't follow what you mean by saying that any task that wants to do this will need to make sure their unpoison attempt was successful. This sort of check would be nicer in a monadic style though (although as mentioned above I really don't know how to make that style rustic.) For example,

atomically $ do
    var $= 0
    is_failed var

@mstewartgallus
Copy link
Contributor Author

Personally, I feel that a RWARC::new_without_poisoning is an ugly approach because it introduces polymorphism of behaviour in a bad, and inelegant way. A better approach might be to make RWARC an interface that could be tightened, or relaxed as needed. For example, a naively written function that took a RWARC could simply take a CheckedRWARC value, and a function that was written, and confirmed not to poison a value could be polymorphic over any value that implemented the RWARC interface. Though this approach would be better, I still think it's silly. Why not go with a simpler approach?

flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 6, 2021
Fix `any()` not taking reference in `search_is_some` lint

`find` gives reference to the item, but `any` does not, so suggestion is broken in some specific cases.

Fixes: rust-lang#7392

changelog: [`search_is_some`] Fix suggestion for `any()` not taking item by reference
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.

3 participants