[go-nuts] Unexpected behaviour when commenting unreachable code

On Tue, Apr 28, 2015 at 4:22 PM, wrote:
Hi everyone!
I've been playing around with golang.org/x/mobile/audio on my desktop pc but
I've experienced an unexpected side effect.
please note that the example I'm pasting reproduces my issue but I don't
think the issue is strictly related to audio/OpenAL, it looks more like
something with the runtime/GC.
but enough with theories, here are the facts: I'm running on amd64 linux, go
1.4.2 64b.

basically audio pkg was not working for me at all, so I went and looked at
the underlying code and OpenAL wrapper and managed to write a working
example.
then I went back to the audio implementation and tried to understand what
was the difference.
here is the code, this code works as expected, it plays a sin wave at 1500Hz
for a couple of seconds.
(I've intentionally stripped all the error checking to make it as concise as
possible for pasting, nothing odd is going on with return values)

package main

import (
"fmt"
"golang.org/x/mobile/audio/al"
"math"
)

func main() {
samplesPerSecond := int64(44100)
frequency := int64(1500)

device := al.Open("")
context := device.CreateContext(nil)
al.MakeContextCurrent(context)

buffers := al.GenBuffers(1)
bytes := make([]byte, 100000)
FillBufferMono8(bytes, samplesPerSecond, frequency)
buffers[0].BufferData(uint32(0x1100), bytes, int32(samplesPerSecond))

sources := al.GenSources(1)
sources[0].QueueBuffers(buffers)
al.PlaySources(sources[0])

waiter := make(chan int)
<-waiter
fmt.Println("you won't see this")

al.DeleteSources(sources[0])
al.DeleteBuffers(buffers)
context.Destroy() // Comment this line
device.Close()
}

func FillBufferMono8(buffer []byte, samplesPerSecond int64, frequency int64)
{
bufferLength := len(buffer)
for i := int64(0); i < int64(bufferLength); i++ {
sampleIndex := i
sinResult := math.Sin(math.Pi * 2 * (float64(sampleIndex%samplesPerSecond) /
float64(samplesPerSecond)) * float64(frequency))
sample := byte(255 * 0.5 * (1.0 + sinResult))
buffer[i] = sample
}
}


fact is, if you comment the line
context.Destroy() // Comment this line
it stops working (no sound played) and behaves exactly as the audio pkg,
giving this output:
AL lib: (WW) FreeContext: (0x8efb20) Deleting 1 Source(s)

as you can see that line comes after a never signaled chan, so it is, for
all practical purposes, unreachable.
the fact that it does or does not exists changes the behaviour of the
program.
Am I missing something? I see the OpenAL wrapper directly interacts with C,
are there some side effect I'm not aware of?

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to [email protected].
For more options, visit https://groups.google.com/d/optout.

On Wed, Apr 29, 2015 at 9:28 AM, Daniele Colla wrote:
thanks David, you are very likely right
I was thinking about something having to do with the GC, and looking at the
sources I completely missed the device.CreateContext function adding a
finalizer to the Context object

but is the GC supposed to be this aggressive? context isn't even out of
scope yet. it looks a bit error prone..
anyway, how would you handle that in the original code?

(ref https://github.com/golang/mobile/blob/master/audio/audio.go)

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to [email protected].
For more options, visit https://groups.google.com/d/optout.

On Wed, Apr 29, 2015 at 6:49 AM, David Crawshaw wrote:
The garbage collector can clean up any unreachable memory. You're in
the scope, but you have no further uses of context.

But your code should work. If a context is current
(al.MakeContextCurrent) and there are valid sources being played
(al.PlaySources), then nothing should be cleaned up automatically. The
al package should be pinning the object and it's not.

If you wouldn't mind, please file an issue and assign it to jbd.