Bug 116 - Crash when removing sampler channel with active streams/voices
Summary: Crash when removing sampler channel with active streams/voices
Status: CLOSED FIXED
Alias: None
Product: LinuxSampler
Classification: Unclassified
Component: gig::Engine (show other bugs)
Version: SVN Trunk
Hardware: PC Linux
: P2 normal
Assignee: Andreas Persson
URL:
Depends on:
Blocks:
 
Reported: 2009-01-25 23:44 CET by Grigor Iliev
Modified: 2009-03-02 15:18 CET (History)
1 user (show)

See Also:


Attachments
116.patch.bz2 (1.18 KB, application/x-bzip)
2009-01-25 23:45 CET, Grigor Iliev
Details
116-2.patch (2.70 KB, patch)
2009-02-22 12:08 CET, Andreas Persson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Grigor Iliev 2009-01-25 23:44:01 CET
When removing sampler channel with active streams/voices the DiskThread is
trying to access already deleted streams.
Christian, please take a look at the proposed fix, because I'm not much familiar
with that part of LS and I may miss something.
Comment 1 Grigor Iliev 2009-01-25 23:45:28 CET
Created attachment 44 [details]
116.patch.bz2
Comment 2 Christian Schoenebeck 2009-01-31 13:10:37 CET
I'm not sure if it's ok just to remove the

Engine::instruments.HandBackDimReg(itVoice->pDimRgn);

loop. I talked with Andreas, he added it due to some problem when he 
implemented the host plugins, he said to be able to load instruments while not 
being connected to an audio output device.

Andreas, ... I leave it to you to decide. ;-)
Comment 3 Andreas Persson 2009-02-22 12:08:07 CET
Created attachment 49 [details]
116-2.patch

I had a look again at this bug (sorry for taking so long).

The first thing I got stuck on in your patch was the change from
HandBackInstrument to HandBack, where you could remove the whole HandBackDimReg
loop. I thought that there must have been a good reason that I wrote that code.
But, now I fail to find such a reason. So, I think it's a good change.

As I said in a mail, a problem with KillImmediatelyAllVoices is that it doesn't
wait for the DiskThread to actually delete the streams.

Then I noticed that DisconnectAudioOutputDevice is calling ResetInternal, which
calls Engine::ResetInternal, which calls DiskThread::Reset, which kills all
streams.

So, I think a simpler solution than your patch is to move the ResetInternal
call first in DisconnectAduioOutputDevice.

I attach my version of the patch. What do you think? Am I missing something?
Comment 4 Grigor Iliev 2009-02-22 15:03:44 CET
(In reply to comment #3)
> Then I noticed that DisconnectAudioOutputDevice is calling ResetInternal, which
> calls Engine::ResetInternal, which calls DiskThread::Reset, which kills all
> streams.

I missed that.

> So, I think a simpler solution than your patch is to move the ResetInternal
> call first in DisconnectAduioOutputDevice.
> 
> I attach my version of the patch. What do you think? Am I missing something?

The only disadvantage I see is that, if we have more sampler channels connected
to the same audio device, Engine::ResetInternal call in
DisconnectAduioOutputDevice will kill all active voices in the other sampler
channels too. But I guess we can live with that :)
The patch looks fine to me.
Comment 5 Andreas Persson 2009-02-23 19:30:24 CET
Patch committed