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

Race condition between createOffer and setIdentityProvider #1184

Closed
taylor-b opened this issue May 9, 2017 · 4 comments · Fixed by #1522
Closed

Race condition between createOffer and setIdentityProvider #1184

taylor-b opened this issue May 9, 2017 · 4 comments · Fixed by #1522

Comments

@taylor-b
Copy link
Contributor

taylor-b commented May 9, 2017

Suppose an application does:

pc.setIdentityProvider(a);
pc.createOffer().then(...);
// Some time later, but before createOffer's promise has resolved:
pc.setIdentityProvider(b);

Is there a guarantee which identity provider will be used to generate "a=identity"?

Note that I already tried to fix the createOffer race conditions here: #875

But this one may still exist.

@martinthomson
Copy link
Member

I think that you have fixed that problem in #875. The process for generating an identity assertion is started synchronously. Provider a is used.

@taylor-b
Copy link
Contributor Author

You're right... But what if you call createOffer again after setIdentityProvider(b)? The second identity assertion request process could complete before the first createOffer call starts generating SDP.

This could be fixed pretty trivially by storing a reference to the identity provider in the synchronous part, and saying "generate an offer according to JSEP, using the identity assertion from provider if provider is defined".

@martinthomson
Copy link
Member

If you change identity provider, the result from the first shouldn't be relevant at the point you call createOffer the second time. The UA can terminate or just ignore the results from the first; the second call will fetch the results from the second one, regardless of whether the first call is still outstanding or completed. After all, at the time of the second call to createOffer, the second identity provider has been set.

In practice, I expect that the first createOffer call will either succeed or fail on its own merits. In Firefox, I think that it succeeds. We don't stop someone from creating multiple offers.

(BTW, I think that in Firefox createOffer could be synchronous, aside from the identity provider calls, so my only real experience with asynchronous processing here is on the identity stuff.)

@fluffy
Copy link
Contributor

fluffy commented May 29, 2017

I could easily live with this race condition not be fixed or specified ... it's not hard for the JS to avoid the problem.

taylor-b added a commit to taylor-b/webrtc-pc that referenced this issue Aug 9, 2017
Fixes w3c#1184.

This cleans things up slightly, and fixes a corner case race condition,
where in the following sequence:

```
pc.setIdentityProvider(a);
pc.createOffer().then(...);
pc.setIdentityProvider(b);
pc.createOffer().then(...);
```

It wasn't completely clear if the first `createOffer` would use the
assertion from `a` or `b`. The answer now is "it will use `a`".
taylor-b added a commit to taylor-b/webrtc-pc that referenced this issue Aug 29, 2017
Fixes w3c#1184.

This cleans things up slightly, and fixes a corner case race condition,
where in the following sequence:

```
pc.setIdentityProvider(a);
pc.createOffer().then(...);
pc.setIdentityProvider(b);
pc.createOffer().then(...);
```

It wasn't completely clear if the first `createOffer` would use the
assertion from `a` or `b`. The answer now is "it will use `a`".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants