gen2brain / avif

AVIF image encoder/decoder
MIT License
27 stars 5 forks source link

Not concurrency safe? #1

Open awenborar opened 1 week ago

awenborar commented 1 week ago

I'm converting avif images to jpeg using the following code:

func FromAvif(r io.Reader) (AwenImage, error) {
    aimg := AwenImage{}

    img, err := avif.Decode(r)
    if err != nil {
        return aimg, err
    }

    buf := bytes.Buffer{}
    j.Encode(&buf, img, &DefaultJpegOptions)
    aimg.Buffer = buf.Bytes()
    aimg.MimeType = "image/jpeg"
    return aimg, nil
}

This works if the function is called sequentially but if I call this function concurrently I receive panics. The panic error is too verbose and I'm not able to pinpoint to the core error.

PS - Great work on the library!

gen2brain commented 1 week ago

If you want to use it concurrently initialize it first (I should probably add Initialize()) with some dummy data, you can use io.Discard, ignore errors, etc. and then you can use it concurrently. The Wazero docs clearly state that this can happen when concurrently initializing the same runtime, and my usage is such that I don't want to do that unconditionally in init().

awenborar commented 1 week ago

I updated my func to run the first call with a lock. Ran 10 images in concurrent batches of 2 - (0,1) then (2, 3) .... The first batch succeeded successfully as it executed sequentially. Now assuming the first batch did the initialization, the second batch should have succeeded, but it didn't.

var avifMw = sync.RWMutex{}
var avifInitialized = false

func FromAvif(r io.Reader) (AwenImage, error) {
    if !avifInitialized {
        avifMw.Lock()
        defer func() {
            avifInitialized = true
            avifMw.Unlock()
        }()
    }

    aimg := AwenImage{}

    img, err := avif.Decode(r)
    if err != nil {
        return aimg, err
    }

    buf := bytes.Buffer{}
    j.Encode(&buf, img, &DefaultJpegOptions)
    aimg.Buffer = buf.Bytes()
    aimg.MimeType = "image/jpeg"
    return aimg, nil
}

I'm refactoring my flow to run all sequentially for now 👍

gen2brain commented 1 week ago

No need to do all of that, just do one Decode with some dummy data that you will ignore errors for, then do whatever you want.

gen2brain commented 1 week ago

There is already something similar in the code, with sync.Once, and it is not enough, similar would be with mutex, try with my suggestion.

awenborar commented 1 week ago

Tried the following:

func init() {
    _, err := FromAvif(bytes.NewBuffer([]byte{}))
    log.Println("avif init expected err", err)
}

func FromAvif(r io.Reader) (AwenImage, error) {
    aimg := AwenImage{}

    img, err := avif.Decode(r)
    if err != nil {
        return aimg, err
    }

    buf := bytes.Buffer{}
    j.Encode(&buf, img, &DefaultJpegOptions)
    aimg.Buffer = buf.Bytes()
    aimg.MimeType = "image/jpeg"
    return aimg, nil
}

This doesn't work either