4 ms·

Does anyone know if OP would be able to work around the single-ownership issue using Cell/RefCell? I still haven't fully grasped how to use those, but they see

Does anyone know if OP would be able to work around the single-ownership issue using Cell/RefCell? I still haven't fully grasped how to use those, but they seem like they'd fit there.

bfrydl 8y ago
OP mentions that the use of unsafe code means it's single-threaded, so yes it's likely that RefCell could be used instead of unsafe code.
- steveklabnik 8y ago
  I'm frankly a bit worried about that statement; I believe it means that they're producing UB. Even with unsafe, Rust code is expected to uphold the safety rules. I haven't read the code though.
  - bfrydl 8y ago
    OP is using unsafe to store a mut pointer to a boxed value so that it can be mutated in two places. So the statement is just referring to data races.
    - steveklabnik 8y ago
      Sounds like UB to me! I’ll have to take a look. It really depends on the details. (I tried to run miri on it, but I'm on Windows, and this is unix-only.) EDIT: even with porting some of the code, I can’t get the current repository to build, there’s a borrow checker error...)
      - MichaelBurge 8y ago
        Sorry about that. If it helps, I'm using Rust 1.32 to build: $ rustc --version rustc 1.32.0 (9fda7c223 2019-01-16) $ cargo --version cargo 1.32.0 (8610973aa 2019-01-02) $ cargo build --release $ cargo run --release --bin nes-emulator `
        steveklabnik 8y ago
        It’s chill! So, two things: 1. I can send you a patch later, but if you use std::io::stdin() and stdout, instead of opening the file descriptors, it should compile on Windows. 2. The borrow error was coming from miri; it detected some UB! Compiling with cargo does work. It’s not the bit I expected; I can file a bug with the error once I’m back at my laptop. Anyway, this is a cool project, and I’m glad you’re doing it; don’t let my worries bug you. I’ve been trying to learn more unsafe stuff and so the UB bits are just on my mind more lately. EDIT: actually it's just from building on nightly rust, not stable. No miri needed. I wonder why stable is okay with this... anyway I filed some github issues, please feel free to ignore me or close them, but if you wanna keep talking about this, let's do that there :)
mtsr 8y ago
Cell and RefCell are both !Sync, as they don't prevent dataraces. You would still need to wrap them in a Mutex to actually make them threadsafe. But Rc<RefCell<Ppu>> would forego the need for unsafe{} here, at the cost of runtime enforcement of unique mutable access, without making it threadsafe. Or Arc<Mutex<Ppu>> for the thread safe variation.

Does anyone know if OP would be able to work around the single-ownership issue using Cell/RefCell? I still haven't fully grasped how to use those, but they seem like they'd fit there.

bfrydl 8y ago

OP mentions that the use of unsafe code means it's single-threaded, so yes it's likely that RefCell could be used instead of unsafe code.

steveklabnik 8y ago
I'm frankly a bit worried about that statement; I believe it means that they're producing UB. Even with unsafe, Rust code is expected to uphold the safety rules. I haven't read the code though.
- bfrydl 8y ago
  OP is using unsafe to store a mut pointer to a boxed value so that it can be mutated in two places. So the statement is just referring to data races.
  - steveklabnik 8y ago
    Sounds like UB to me! I’ll have to take a look. It really depends on the details. (I tried to run miri on it, but I'm on Windows, and this is unix-only.) EDIT: even with porting some of the code, I can’t get the current repository to build, there’s a borrow checker error...)
    - MichaelBurge 8y ago
      Sorry about that. If it helps, I'm using Rust 1.32 to build: $ rustc --version rustc 1.32.0 (9fda7c223 2019-01-16) $ cargo --version cargo 1.32.0 (8610973aa 2019-01-02) $ cargo build --release $ cargo run --release --bin nes-emulator `
      - steveklabnik 8y ago
        It’s chill! So, two things: 1. I can send you a patch later, but if you use std::io::stdin() and stdout, instead of opening the file descriptors, it should compile on Windows. 2. The borrow error was coming from miri; it detected some UB! Compiling with cargo does work. It’s not the bit I expected; I can file a bug with the error once I’m back at my laptop. Anyway, this is a cool project, and I’m glad you’re doing it; don’t let my worries bug you. I’ve been trying to learn more unsafe stuff and so the UB bits are just on my mind more lately. EDIT: actually it's just from building on nightly rust, not stable. No miri needed. I wonder why stable is okay with this... anyway I filed some github issues, please feel free to ignore me or close them, but if you wanna keep talking about this, let's do that there :)

mtsr 8y ago

Cell and RefCell are both !Sync, as they don't prevent dataraces. You would still need to wrap them in a Mutex to actually make them threadsafe. But Rc<RefCell<Ppu>> would forego the need for unsafe{} here, at the cost of runtime enforcement of unique mutable access, without making it threadsafe. Or Arc<Mutex<Ppu>> for the thread safe variation.