soloud-rs icon indicating copy to clipboard operation
soloud-rs copied to clipboard

Memory violation if dropping Wav after Soloud.

Open TKFRvisionOfficial opened this issue 1 year ago • 2 comments

Describe the bug If a Soloud instance gets fed a Wav Object and the Soloud instance is dropped. The drop function of the Wav functions will freeze and crash the application. This is a problem when creating structures that contain instances of both types in the wrong order.

To Reproduce

use soloud::{AudioExt, Soloud, Wav};

fn main() {
    let mut sl = Soloud::default().unwrap();
    let wav = Wav::default();

    sl.play(&wav);

    std::mem::drop(sl);
    std::mem::drop(wav);  // This is where the application will crash.

    println!("We will never reach this print");
}

Expected behavior The dropping of wav shouldn't freeze and then crash the program.

Desktop (please complete the following information):

  • OS: Windows 11
  • Version: 23H2

cargo build -vv

Fresh shlex v1.3.0
Fresh cc v1.2.1
Fresh bitflags v2.6.0
Fresh cmake v0.1.51
Fresh paste v1.0.15
Fresh soloud-sys v1.0.5
Fresh soloud v1.0.5
Fresh audiocrash v0.1.0

TKFRvisionOfficial avatar Nov 18 '24 17:11 TKFRvisionOfficial

The issue comes from the fact that after calling the play function, a pointer to the Soloud object gets set to the Wav object as a member https://github.com/jarikomppa/soloud/blob/e82fd32c1f62183922f08c14c814a02b58db1873/src/core/soloud_core_basicops.cpp#L42. In the deconstructor of the Wav Object stop gets called https://github.com/jarikomppa/soloud/blob/e82fd32c1f62183922f08c14c814a02b58db1873/src/audiosource/wav/soloud_wav.cpp#L88 which in turn is trying to stop the audio source from the Soloud object https://github.com/jarikomppa/soloud/blob/e82fd32c1f62183922f08c14c814a02b58db1873/src/core/soloud_audiosource.cpp#L275. Since the Soloud object is already deallocated there is an illegal memory access.

TKFRvisionOfficial avatar Nov 18 '24 17:11 TKFRvisionOfficial

Thank you for the detailed report.

The only solution I can currently think of is to remove Soloud_destroy from the Drop impl of the Soloud object, and only keep Soloud_deinit (which doesn't cause a crash or heap corruption). And make destroying the Soloud object an explicit and unsafe function.

MoAlyousef avatar Nov 19 '24 07:11 MoAlyousef