Working around overzealous drop check

This playground shows the shape of the code I'm writing. It compiles fine when the _nontrivial_drop field of Encoder is commented out. However, when that field is added, we get some errors from the drop check (I think):

error[E0499]: cannot borrow `*stuff` as mutable more than once at a time
  --> src/lib.rs:58:57
   |
51 |         stuff,
   |         ----- first mutable borrow occurs here
...
56 |             encoder = match encoder.block() {
   |             ------- first borrow might be used here, when `encoder` is dropped and runs the destructor for type `Encoder<'_>`
57 |                 Either::Left(enc) => break enc,
58 |                 Either::Right(save) => save.resume(grow(stuff)),
   |                                                         ^^^^^ second mutable borrow occurs here

error[E0499]: cannot borrow `*stuff` as mutable more than once at a time
  --> src/lib.rs:64:57
   |
51 |         stuff,
   |         ----- first mutable borrow occurs here
...
61 |         encoder = loop {
   |         ------- first borrow might be used here, when `encoder` is dropped and runs the destructor for type `Encoder<'_>`
...
64 |                 Either::Right(save) => save.resume(grow(stuff)),
   |                                                         ^^^^^ second mutable borrow occurs here

For more information about this error, try `rustc --explain E0499`.

Is there actually a soundness issue here? I wouldn't think so, since the drop glue for Encoder<'a> won't touch the &'a mut [u8] field. And if my code is sound, how can I (safely or unsafely) work around the drop check in this case? (I don't think the eyepatch helps.)

1 Like

You write functions to return the item you intend to still use. e.g.
fn block(self) -> Either<BlockEncoder<'a>, (EncoderSave, &'a mut Vec<u8>)> {

1 Like

I’ve done some reduction work to get a smaller example; here’s what I got so far:

#![allow(dead_code)]

enum Either<L, R> {
    Left(L),
    Right(R),
}

struct Encoder<'a, T> {
    stuff: &'a mut [u8],
    _nontrivial_drop: T,
}

struct EncoderSave<T>(T);

impl<T> EncoderSave<T> {
    fn resume<'a>(self, _stuff: &'a mut [u8]) -> Encoder<'a, T> {
        unimplemented!()
    }
}

impl<'a, T> Encoder<'a, T> {
    fn block(self) -> Either<(), EncoderSave<T>> {
        unimplemented!()
    }
}

fn grow(_: &mut Vec<u8>) -> &mut [u8] {
    unimplemented!()
}

// fails without the `: Copy`

// but then, works again, if the out-of-loop `let mut encoder`
// becomes an ordinary `let encoder =` inside of the loop
fn branch1<T: Copy>(mut save: EncoderSave<T>, stuff: &mut Vec<u8>) -> () {
    let mut encoder;
    loop {
        encoder = save.resume(grow(stuff));
        save = match encoder.block() {
            Either::Left(enc) => break,
            Either::Right(save) => save,
        };
    };
}
1 Like

One workaround is to create loop-local Encoders.

1 Like

Maybe this?
https://github.com/rust-lang/rust/issues/70919

2 Likes

It’s not pretty or particularly DRY, but this compiles even though the control flow is essentially “the same”; probably because it successfully avoids mutating encoder or block_encoder in any loop that also involves repeated .resume(grow(stuff)) calls:

#![allow(dead_code)]

enum Either<L, R> {
    Left(L),
    Right(R),
}

struct Encoder<'a> {
    stuff: &'a mut [u8],
    _nontrivial_drop: Vec<u8>,
}

struct EncoderSave;

impl EncoderSave {
    fn resume(self, _stuff: &mut [u8]) -> Encoder<'_> {
        unimplemented!()
    }
}

impl<'a> Encoder<'a> {
    fn block(self) -> Either<BlockEncoder<'a>, EncoderSave> {
        unimplemented!()
    }
}

struct BlockEncoder<'a> {
    stuff: &'a mut [u8],
    _nontrivial_drop: Vec<u8>,
}

struct BlockEncoderSave;

impl BlockEncoderSave {
    fn resume(self, _stuff: &mut [u8]) -> BlockEncoder<'_> {
        unimplemented!()
    }
}

impl<'a> BlockEncoder<'a> {
    fn encode(self) -> Either<Encoder<'a>, BlockEncoderSave> {
        unimplemented!()
    }
}

fn grow(_: &mut Vec<u8>) -> &mut [u8] {
    unimplemented!()
}

fn go(stuff: &mut Vec<u8>) {
    let mut encoder = Encoder {
        stuff,
        _nontrivial_drop: Vec::new(),
    };
    let mut block_encoder;
    loop {
        match encoder.block() {
            Either::Left(enc) => {
                block_encoder = enc;
            }
            Either::Right(mut save) => {
                let mut bsave;
                loop {
                    'x: loop {
                        let mut encoder = save.resume(grow(stuff));
                        let mut block_encoder;
                        loop {
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    continue 'x;
                                }
                            }
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    break 'x;
                                }
                            }
                        }
                    }
                    'y: loop {
                        let mut encoder;
                        let mut block_encoder = bsave.resume(grow(stuff));
                        loop {
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    continue 'y;
                                }
                            }
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    break 'y;
                                }
                            }
                        }
                    }
                }
            }
        }
        match block_encoder.encode() {
            Either::Left(enc) => {
                encoder = enc;
            }
            Either::Right(mut bsave) => {
                let mut save;
                loop {
                    'y: loop {
                        let mut encoder;
                        let mut block_encoder = bsave.resume(grow(stuff));
                        loop {
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    continue 'y;
                                }
                            }
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    break 'y;
                                }
                            }
                        }
                    }
                    'x: loop {
                        let mut encoder = save.resume(grow(stuff));
                        let mut block_encoder;
                        loop {
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    continue 'x;
                                }
                            }
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    break 'x;
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

In terms of practical workarounds: this seems to work nicely and without overhead:

fn go(stuff: &mut Vec<u8>) {
    let mut encoder = ManuallyDrop::new(Encoder {
        stuff,
        _nontrivial_drop: Vec::new(),
    });
    loop {
        let mut block_encoder = loop {
            encoder = ManuallyDrop::new(match ManuallyDrop::into_inner(encoder).block() {
                Either::Left(enc) => break enc,
                Either::Right(save) => save.resume(grow(stuff)),
            });
        };
        encoder = loop {
            block_encoder = match block_encoder.encode() {
                Either::Left(enc) => break ManuallyDrop::new(enc),
                Either::Right(save) => save.resume(grow(stuff)),
            };
        };
    }
}

Might need block_encoder wrapped similarly if that one has nontrivial drop, too.


There isn’t even any downsides such as leak on panic, since the encoder is always immediately consumed with …::into_inner without anything that can panic in-between. I think. It's probably easy to restructure the code in order to make this even more obvious.

3 Likes

(Looking over my code again I'm struck by the bad/confusing type and variable names. Sorry about that.)

Thanks, this seems promising. I'll report back whether I'm able to apply it to my real code.

I need to study this a bit more before I can wrap my head around it. It seems like the key is to temporarily split the nontrivial-Drop part of Encoder from the &mut part, so the compiler can't see problematic interaction between them.

Looks familiar. I guess I misidentified the issue in my OP—the destructor of Encoder<'a> never runs at all, because the value is consumed by Encoder::block instead, but the compiler doesn't see that for whatever reason.

Neat! I'd love to see whether the core logic here can be packaged up into a utility method on Encoder or something, because ultimately I want consumers of my library to be able to write this kind of loop in their own code.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.