Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CachedFontFace seems to be causing crash #3134

Closed
krbs opened this issue Jul 11, 2022 · 1 comment · Fixed by #3135
Closed

CachedFontFace seems to be causing crash #3134

krbs opened this issue Jul 11, 2022 · 1 comment · Fixed by #3135
Labels
bug Something isn't working

Comments

@krbs
Copy link
Contributor

krbs commented Jul 11, 2022

Fyne v2.2.3, Windows 11/21H2, go1.17.11. Crash happens if multiple faces of the same font is used.

Stacktrace:

fatal error: concurrent map writes

goroutine 138 [running]:
runtime.throw({0x21f66c0, 0x48})
        C:/Program Files/Go/src/runtime/panic.go:1198 +0x76 fp=0xc0008bd3e8 sp=0xc0008bd3b8 pc=0x2a83f6
runtime.mapassign(0xc0000f85a0, 0xc0008bd5d8, 0xc0008bd490)
        C:/Program Files/Go/src/runtime/map.go:585 +0x4d6 fp=0xc0008bd468 sp=0xc0008bd3e8 pc=0x280516
fyne.io/fyne/v2/internal/painter.CachedFontFace({0x0, 0x0, 0x0, 0x0, 0x0}, 0xc0008bd5d8)
        C:/Users/go/pkg/mod/fyne.io/fyne/v2@v2.2.3/internal/painter/font.go:70 +0x46d fp=0xc0008bd518 sp=0xc0008bd468 pc=0x7884cd

Safeguarding it with RWMutex resolves it.

        comp := val.(*fontCacheItem)
        comp.facesMutex.RLock()
        face := comp.faces[*opts]
        comp.facesMutex.RUnlock()
        if face == nil {
                f1 := truetype.NewFace(comp.font, opts)
                f2 := truetype.NewFace(comp.fallback, opts)
                face = newFontWithFallback(f1, f2, comp.font, comp.fallback)

                comp.facesMutex.Lock()
                comp.faces[*opts] = face
                comp.facesMutex.Unlock()
        }
type fontCacheItem struct {
        font, fallback *truetype.Font
        faces          map[truetype.Options]font.Face
        facesMutex     sync.RWMutex
}

Race was introduced in this commit: d83ffac

@krbs krbs added the unverified A bug that has been reported but not verified label Jul 11, 2022
@krbs krbs mentioned this issue Jul 11, 2022
3 tasks
@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Jul 15, 2022
@andydotxyz andydotxyz added this to the Fixes (v2.2.x) milestone Jul 15, 2022
@andydotxyz
Copy link
Member

Fixed on develop and release branch, ready for next release (v2.3 or v2.2.4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants