search

swami / libinstpatch

Instrument file software library.
Other
20 stars 6 forks source link

fix memory violation access on swap file for Windows. #51

Closed jjceresa closed 4 years ago

jjceresa commented 4 years ago

Using g_open()and g_file_open_tmp() leads to memory violation access on next call to lseek.

This PR fixes these issues by replacing call to g_open()and g_file_open_tmp() by call to open().

jjceresa commented 4 years ago

Actually on Windows Using g_open() and g_file_open_tmp() leads to memory violation access on next call to lseek.

I don't know if this issue is only specific on Windows due to the glib in use (v 2.28.8) ?. In order to verify if this issue occurs (or not) on linux, this can be easily done using the attached soundfont acoustic_piano_imis_1.sf2 acoustic_piano_imis_1.zip (in next commit too). On swami, following the steps: 1) File Open soundfont acoustic_piano_imis_1.sf2 Please ignore the bunch of warnings WARNING : Sample 'piano_mf_c1_l' has invalid loop... These warnings aren't relevant. 2) Select Melodic Presets- 000-000 acoustic piano (we see the green split range to the right pane below the piano gui) 3) Close the soundfont:Right clickon SF2 Acoustic Piano then choose Close. On Windows, this leads to a violation access (described in commit https://github.com/swami/libinstpatch/pull/51/commits/73372399487c1993fb46925a79cf18a30f0021ed above ).

derselbst commented 4 years ago

Linux works without issues. But on win10 x64 I can reproduce this:

image

Strangely, VisualStudio doesn't give me a call stack. No clue why this happens. Glib claims it only calls open() under the hood, so this should work. Couldn't find a related bug report to glib either.

So, I would take your fix. But pls, why did you add that soundfont to version control?

jjceresa commented 4 years ago

Strangely, VisualStudio doesn't give me a call stack. No clue why this happens.

Yes, here I was facing to the same result using VS2010, (although now I just realize I haven' yet tried in Debug release).

But pls, why did you add that soundfont to version control?

Sorry, this is because I was not sure that direct uploading would work. I made a dumpling, stupid of me !. Now I have polluted this branch with this sf2, what could be the best way to get rid of commit ccf5dd8 ?. I have made a new PR 52 with a new branch swap-file-2. Hope this is fine ?

derselbst commented 4 years ago

Yes, that's fine. Thanks.