Skip to content

Conversation

antonilol
Copy link
Contributor

Breaking changes:

  • mixer::Chunk fields were made private because the struct relied on these pointer being valid, but users could set them to any value
  • Timer callback must live for 'static because the callback can be called after the scope ends (by using std::mem::forget).

timer ub reproduction (ran from a #[test]):

let sdl_context = crate::sdl::init().unwrap();
let timer_subsystem = sdl_context.timer().unwrap();

let mut v = Box::new(123);

std::mem::forget(timer_subsystem.add_timer(
    20,
    Box::new(|| {
        dbg!(std::mem::take(&mut v));
        0
    }),
));

drop(v);

// v is used after dropping (from the timer callback)

std::thread::sleep(Duration::from_millis(30));

Comment on lines -124 to -127
// I tried using `std::panic::catch_unwind()` here and it compiled but
// would not catch. Maybe wait for `c_unwind` to stabilize? Then the behavior
// will automatically abort the process when panicking over an `extern "C"`
// function.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why that did not work, it should work.

Since Rust 1.81, unwinding in an extern "C" function will abort the process, I added the catch_unwind to not have this make undefined behavior on "older" (1.81 is almost 2 weeks old at the time of writing this comment) versions and have the same behavior independent of the Rust version.

@antonilol antonilol force-pushed the fix-ub branch 2 times, most recently from 7ff5c32 to 10b888e Compare September 20, 2024 21:51
@antonilol antonilol marked this pull request as ready for review September 20, 2024 21:55
@antonilol antonilol changed the title Draft: fix ub, shrink unsafe blocks Fix ub and shrink unsafe blocks Sep 20, 2024
@antonilol antonilol requested a review from jagprog5 March 18, 2025 15:26
@antonilol antonilol merged commit 5be34ce into Rust-SDL2:master Mar 20, 2025
17 checks passed
@antonilol antonilol deleted the fix-ub branch March 20, 2025 13:07
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.

2 participants