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

Possible race condition in playCtrl #48

Closed
veloce opened this issue Jan 14, 2015 · 7 comments
Closed

Possible race condition in playCtrl #48

veloce opened this issue Jan 14, 2015 · 7 comments
Assignees
Labels

Comments

@veloce
Copy link
Collaborator

veloce commented Jan 14, 2015

this.round and this.gameSocket are initialised at null and an asynchronous xhr populates them. But the view depends on those objects.

They should be created before and be playCtrl parameters to avoid problems.

It doesn't respect the "model must always be valid" principle.

@veloce veloce added the bug label Jan 14, 2015
@veloce veloce self-assigned this Jan 14, 2015
@veloce
Copy link
Collaborator Author

veloce commented Jan 14, 2015

Maybe instead of creating a new instance of StrongSocket for each game we should keep a global one and pass it to each game controller.

@ornicar
Copy link
Collaborator

ornicar commented Jan 14, 2015

But sockets are created for a URL that contains the game ID?

@veloce
Copy link
Collaborator Author

veloce commented Jan 14, 2015

We could add the id param to StrongSocket.reset since we already reset version.
So we'd create 2 instances of StrongSocket one for game, one for lobby, on app init.

What do you think?

Also playCtrl should be removed completely as there's nothing left here... all should be done in round

@ornicar
Copy link
Collaborator

ornicar commented Jan 14, 2015

I don't think you can reuse a socket connection for distinct games. A round socket is bound to one game, defined by the round socket URL.

@ornicar
Copy link
Collaborator

ornicar commented Jan 14, 2015

nevermind, there's a confusion between instances of StrongSocket and actual browser WS connection.

@ornicar
Copy link
Collaborator

ornicar commented Jan 14, 2015

Yes reset could have an URL aargument additionally to version, to allow reuse of the StrongSocket instance while changing the underlying connection

@veloce
Copy link
Collaborator Author

veloce commented Jan 14, 2015

Yep that's what I meant. I think this is a fair approach to the problem mentioned above. And maybe it'll solve other issues...

@veloce veloce closed this as completed Jan 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants