Jesse Schwartzentruber (:truber) Reporter
	Description • 7 years ago

The attached testcase causes various gtk3 crashes when stylo is enabled by pref.

I'll attach logs for the various signatures I've been able to capture so far.
* pc = 0
* pc = wild
* segv reading wild ptr
* segv reading 0
* gtk abort
* double-free (infrequent)
* heap-use-after-free (once)

I also saw double-free infrequently and heap-use-after-free once during reduction. The segv on wild ptr and gtk abort were the most common.

This was hard to reduce, so it's not very concise and may still be intermittent.

	Jesse Schwartzentruber (:truber) Reporter
	Comment 1 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 2 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 3 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 4 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 5 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 6 • 7 years ago

in m-c rev 20170817-a6a1f5c1d971

	Jesse Schwartzentruber (:truber) Reporter
	Comment 7 • 7 years ago

in m-i rev 20170812-113580d267ea

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 8 • 7 years ago

Yeah, sounds like GTK may be doing something non-threadsafe there, and stylo is just triggering it... :(

	Jesse Schwartzentruber (:truber) Reporter
	Comment 9 • 7 years ago

Btw my GTK version is: 3.22.18

	Andrew McCreight [:mccr8]
	Comment 10 • 7 years ago

FWIW, bug 1386915 also involved nsLookAndFeel::NativeGetColor.

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 11 • 7 years ago

Just checked gtk's source, and there's lots of non thread-safe stuff going on, from non-atomic refcounts, to global caches. I think I'd just stash a mutex there, not much else we can do :(

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 12 • 7 years ago

This does the conservative thing, and fixes the crash for me as expected.

I doubt this will affect performance noticeably, since system colors are uncommon in content pages at least, and the chances on racing computing them are unlikely.

We can do more fine-grained stuff if we need to I suppose.

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 13 • 7 years ago

remote: added 1 changesets with 1 changes to 1 files
remote: recorded push in pushlog
remote: 
remote: View your change here:
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/de22ecdc7ec67f2e6e4186d9f390a73f93e28a53
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=de22ecdc7ec67f2e6e4186d9f390a73f93e28a53

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 14 • 7 years ago

Cam, do you know what's the best way to add a test-case for this? It continually reloads the page, so it'll timeout in normal builds, do we have something like that?

Maybe stashing it in an iframe and reloading it a few times?

	Cameron McCormack (:heycam)
	Comment 15 • 7 years ago

If it would normally reproduce after a few reloads, then yeah, reloading an iframe a few times should be OK.  Although if it did start failing it would do so intermittently, which wouldn't be great I guess.  If it's not easy to reproduce the problem then feel free not to add a test.

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 7 years ago

https://hg.mozilla.org/mozilla-central/rev/de22ecdc7ec6