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

Use a Once per Player init #177

Merged
merged 1 commit into from Jan 10, 2019
Merged

Conversation

@ferjm
Copy link
Member

ferjm commented Jan 9, 2019

We need an init flag per Player instance, not a global flag. Regarding this question I believe the usage of Ordering::Relaxed is fine here as it is harmless to send the unlock message twice.

@ferjm
Copy link
Member Author

ferjm commented Jan 9, 2019

@ferjm
Copy link
Member Author

ferjm commented Jan 9, 2019

This is causing a bunch of WPTs timeouts servo/servo#22522 (comment)

@Manishearth
Copy link
Member

Manishearth commented Jan 9, 2019

Why not just make is_ready a Once? The point of suggesting Once wasn't to suggest a global, it was to provide a better alternative to manual atomics

@ferjm ferjm force-pushed the ferjm:do.not.use.once.player.init branch from 5a433f3 to 5faa03d Jan 10, 2019
@ferjm
Copy link
Member Author

ferjm commented Jan 10, 2019

@Manishearth done. Not sure if I can get rid of the Arc wrapping the Once.

@ferjm ferjm changed the title Revert "Use Once for player initialization" Use a Once per Player init Jan 10, 2019
@Manishearth
Copy link
Member

Manishearth commented Jan 10, 2019

r+

travis doesn't seem to like the PR

@ferjm
Copy link
Member Author

ferjm commented Jan 10, 2019

Yeah, Travis is broken since the migration. I don't know exactly why yet.

@ferjm ferjm merged commit c8cc491 into servo:master Jan 10, 2019
1 check failed
1 check failed
Travis CI - Pull Request Build Errored
Details
@ferjm ferjm deleted the ferjm:do.not.use.once.player.init branch Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.