Skip to content

Commit a6fc9d3

Browse files
kannan-xiao4kannan-xiao4
authored
Fix: webserver error (#497)
* check exsit pair before access on offer/answer * fix test * check client exist when delete connection * fix delete process * fix and add test about ReNegotiationAfterReceivingFirstOffer * wait while not stable on sendoffercorutine * controll offer answer order * wait to stable on manage
1 parent 2de1fde commit a6fc9d3

File tree

10 files changed

+149
-66
lines changed

10 files changed

+149
-66
lines changed

‎WebApp/public/bidirectional/js/peer.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export default class Peer extends EventTarget {
6868
async loopResendOffer() {
6969

7070
while (true) {
71-
if(this.waitingAnswer) {
71+
if(this.pc != null && this.waitingAnswer) {
7272
this.dispatchEvent(new CustomEvent('sendoffer', { detail: { connectionId: this.connectionId, sdp: this.pc.localDescription.sdp } }));
7373
}
7474
await this.sleep(this.interval);

‎WebApp/public/bidirectional/js/sendvideo.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class SendVideo {
3939

4040
this.signaling.addEventListener('disconnect', async (e) => {
4141
const data = e.detail;
42-
if (_this.pc.connectionId == data.connectionId) {
42+
if (_this.pc != null && _this.pc.connectionId == data.connectionId) {
4343
_this.ondisconnect();
4444
}
4545
});
@@ -57,13 +57,17 @@ export class SendVideo {
5757
this.signaling.addEventListener('answer', async (e) => {
5858
const answer = e.detail;
5959
const desc = new RTCSessionDescription({ sdp: answer.sdp, type: "answer" });
60-
await _this.pc.onGotDescription(answer.connectionId, desc);
60+
if (_this.pc != null) {
61+
await _this.pc.onGotDescription(answer.connectionId, desc);
62+
}
6163
});
6264

6365
this.signaling.addEventListener('candidate', async (e) => {
6466
const candidate = e.detail;
6567
const iceCandidate = new RTCIceCandidate({ candidate: candidate.candidate, sdpMid: candidate.sdpMid, sdpMLineIndex: candidate.sdpMLineIndex });
66-
await _this.pc.onGotCandidate(candidate.connectionId, iceCandidate);
68+
if (_this.pc != null) {
69+
await _this.pc.onGotCandidate(candidate.connectionId, iceCandidate);
70+
}
6771
});
6872

6973
await this.signaling.start();

‎WebApp/public/js/signaling.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export default class Signaling extends EventTarget {
55
constructor() {
66
super();
77
this.interval = 3000;
8+
this.running = false;
89
this.sleep = msec => new Promise(resolve => setTimeout(resolve, msec));
910
}
1011

@@ -25,6 +26,7 @@ export default class Signaling extends EventTarget {
2526
const createResponse = await fetch(this.url(''), { method: 'PUT', headers: this.headers() });
2627
const session = await createResponse.json();
2728
this.sessionId = session.sessionId;
29+
this.running = true;
2830

2931
this.loopGetOffer();
3032
this.loopGetAnswer();
@@ -34,7 +36,7 @@ export default class Signaling extends EventTarget {
3436
async loopGetOffer() {
3537
let lastTimeRequest = Date.now() - 30000;
3638

37-
while (true) {
39+
while (this.running) {
3840
const res = await this.getOffer(lastTimeRequest);
3941
lastTimeRequest = Date.parse(res.headers.get('Date'));
4042

@@ -54,7 +56,7 @@ export default class Signaling extends EventTarget {
5456
// receive answer message from 30secs ago
5557
let lastTimeRequest = Date.now() - 30000;
5658

57-
while (true) {
59+
while (this.running) {
5860
const res = await this.getAnswer(lastTimeRequest);
5961
lastTimeRequest = Date.parse(res.headers.get('Date'));
6062

@@ -74,7 +76,7 @@ export default class Signaling extends EventTarget {
7476
// receive answer message from 30secs ago
7577
let lastTimeRequest = Date.now() - 30000;
7678

77-
while (true) {
79+
while (this.running) {
7880
const res = await this.getCandidate(lastTimeRequest);
7981
lastTimeRequest = Date.parse(res.headers.get('Date'));
8082

@@ -93,6 +95,7 @@ export default class Signaling extends EventTarget {
9395
}
9496

9597
async stop() {
98+
this.running = false;
9699
await fetch(this.url(''), { method: 'DELETE', headers: this.headers() });
97100
this.sessionId = null;
98101
}

‎WebApp/src/signaling.ts

+21-13
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,16 @@ router.delete('/connection', (req: Request, res: Response) => {
180180
const pair = connectionPair.get(connectionId);
181181
const otherSessionId = pair[0] == sessionId ? pair[1] : pair[0];
182182
if (otherSessionId) {
183-
clients.get(otherSessionId).delete(connectionId);
183+
if (clients.has(otherSessionId)) {
184+
clients.get(otherSessionId).delete(connectionId);
185+
}
184186
}
185187
}
186188
connectionPair.delete(connectionId);
187189
offers.get(sessionId).delete(connectionId);
188190
answers.get(sessionId).delete(connectionId);
189191
candidates.get(sessionId).delete(connectionId);
190-
res.sendStatus(200);
192+
res.json({ connectionId: connectionId });
191193
});
192194

193195
router.post('/offer', (req: Request, res: Response) => {
@@ -197,20 +199,21 @@ router.post('/offer', (req: Request, res: Response) => {
197199
let polite = false;
198200

199201
if (res.app.get('isPrivate')) {
200-
const pair = connectionPair.get(connectionId);
201-
keySessionId = pair[0] == sessionId ? pair[1] : pair[0];
202-
if (keySessionId == null) {
203-
const err = new Error(`${connectionId}: This connection id is not ready other session.`);
204-
console.log(err);
205-
res.status(400).send({ error: err });
206-
return;
202+
if (connectionPair.has(connectionId)) {
203+
const pair = connectionPair.get(connectionId);
204+
keySessionId = pair[0] == sessionId ? pair[1] : pair[0];
205+
if (keySessionId != null) {
206+
polite = true;
207+
const map = offers.get(keySessionId);
208+
map.set(connectionId, new Offer(req.body.sdp, Date.now(), polite))
209+
}
207210
}
208-
polite = true;
209-
} else {
210-
connectionPair.set(connectionId, [sessionId, null]);
211-
keySessionId = sessionId;
211+
res.sendStatus(200);
212+
return;
212213
}
213214

215+
connectionPair.set(connectionId, [sessionId, null]);
216+
keySessionId = sessionId;
214217
const map = offers.get(keySessionId);
215218
map.set(connectionId, new Offer(req.body.sdp, Date.now(), polite))
216219

@@ -223,6 +226,11 @@ router.post('/answer', (req: Request, res: Response) => {
223226
const connectionIds = getOrCreateConnectionIds(sessionId);
224227
connectionIds.add(connectionId);
225228

229+
if (!connectionPair.has(connectionId)) {
230+
res.sendStatus(200);
231+
return;
232+
}
233+
226234
// add connectionPair
227235
const pair = connectionPair.get(connectionId);
228236
const otherSessionId = pair[0] == sessionId ? pair[1] : pair[0];

‎WebApp/src/websocket.ts

+21-29
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,13 @@ export default class WSSignaling {
134134
let newOffer = new Offer(message.sdp, Date.now(), false);
135135

136136
if (this.isPrivate) {
137-
const pair = connectionPair.get(connectionId);
138-
const otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
139-
if (otherSessionWs) {
140-
newOffer.polite = true;
141-
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "offer", data: newOffer }));
142-
} else {
143-
ws.send(JSON.stringify({ type: "error", message: `${connectionId}: This connection id is not ready other session.` }));
137+
if (connectionPair.has(connectionId)) {
138+
const pair = connectionPair.get(connectionId);
139+
const otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
140+
if (otherSessionWs) {
141+
newOffer.polite = true;
142+
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "offer", data: newOffer }));
143+
}
144144
}
145145
return;
146146
}
@@ -160,39 +160,31 @@ export default class WSSignaling {
160160
connectionIds.add(connectionId);
161161
const newAnswer = new Answer(message.sdp, Date.now());
162162

163-
let otherSessionWs = null;
164-
165-
if (this.isPrivate) {
166-
const pair = connectionPair.get(connectionId);
167-
otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
168-
} else {
169-
const pair = connectionPair.get(connectionId);
170-
otherSessionWs = pair[0];
171-
connectionPair.set(connectionId, [otherSessionWs, ws]);
163+
if (!connectionPair.has(connectionId)) {
164+
return;
172165
}
173166

174-
if (this.isPrivate) {
175-
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "answer", data: newAnswer }));
176-
return;
167+
const pair = connectionPair.get(connectionId);
168+
const otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
169+
170+
if (!this.isPrivate) {
171+
connectionPair.set(connectionId, [otherSessionWs, ws]);
177172
}
178173

179-
clients.forEach((_v, k) => {
180-
if (k == ws) {
181-
return;
182-
}
183-
k.send(JSON.stringify({ from: connectionId, to: "", type: "answer", data: newAnswer }));
184-
});
174+
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "answer", data: newAnswer }));
185175
}
186176

187177
private onCandidate(ws: WebSocket, message: any) {
188178
const connectionId = message.connectionId;
189179
const candidate = new Candidate(message.candidate, message.sdpMLineIndex, message.sdpMid, Date.now());
190180

191181
if (this.isPrivate) {
192-
const pair = connectionPair.get(connectionId);
193-
const otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
194-
if (otherSessionWs) {
195-
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "candidate", data: candidate }));
182+
if (connectionPair.has(connectionId)) {
183+
const pair = connectionPair.get(connectionId);
184+
const otherSessionWs = pair[0] == ws ? pair[1] : pair[0];
185+
if (otherSessionWs) {
186+
otherSessionWs.send(JSON.stringify({ from: connectionId, to: "", type: "candidate", data: candidate }));
187+
}
196188
}
197189
return;
198190
}

‎com.unity.renderstreaming/Runtime/Scripts/RenderStreamingInternal.cs

+10-4
Original file line numberDiff line numberDiff line change
@@ -396,13 +396,18 @@ PeerConnection CreatePeerConnection(string connectionId, bool polite)
396396
{
397397
onAddReceiver?.Invoke(connectionId, trackEvent.Receiver);
398398
};
399-
pc.OnNegotiationNeeded = () => OnNegotiationNeeded(connectionId);
399+
pc.OnNegotiationNeeded = () => _startCoroutine(OnNegotiationNeeded(connectionId));
400400
return peer;
401401
}
402402

403403
void DeletePeerConnection(string connectionId)
404404
{
405-
_mapConnectionIdAndPeer[connectionId].Dispose();
405+
if (!_mapConnectionIdAndPeer.TryGetValue(connectionId, out var peer))
406+
{
407+
return;
408+
}
409+
410+
peer.Dispose();
406411
_mapConnectionIdAndPeer.Remove(connectionId);
407412
}
408413

@@ -424,15 +429,16 @@ void OnIceConnectionChange(string connectionId, RTCIceConnectionState state)
424429
}
425430
}
426431

427-
void OnNegotiationNeeded(string connectionId)
432+
IEnumerator OnNegotiationNeeded(string connectionId)
428433
{
434+
yield return new WaitWhile(() => !IsStable(connectionId));
429435
SendOffer(connectionId);
430436
}
431437

432438
IEnumerator SendOfferCoroutine(string connectionId, PeerConnection pc)
433439
{
434440
// waiting other setLocalDescription process
435-
yield return new WaitWhile(() => pc.makingOffer || pc.makingAnswer);
441+
yield return new WaitWhile(() => !IsStable(connectionId));
436442

437443
Assert.AreEqual(pc.peer.SignalingState, RTCSignalingState.Stable,
438444
$"{pc} negotiationneeded always fires in stable state");

‎com.unity.renderstreaming/Runtime/Scripts/Signaling/HttpSignaling.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,9 @@ private void HTTPPooling()
150150
try
151151
{
152152
HTTPGetConnections();
153-
HTTPGetOffers();
153+
//ToDo workaround: The processing order needs to be determined by the time stamp
154154
HTTPGetAnswers();
155+
HTTPGetOffers();
155156
HTTPGetCandidates();
156157

157158
Thread.Sleep((int)(m_timeout * 1000));

‎com.unity.renderstreaming/Tests/Runtime/PrivateSignalingTest.cs

-6
Original file line numberDiff line numberDiff line change
@@ -288,12 +288,6 @@ public IEnumerator OnOffer()
288288
yield return new WaitUntil(() => !string.IsNullOrEmpty(connectionId1));
289289

290290
signaling2.OnOffer += (s, e) => { offerRaised2 = true; };
291-
292-
if (m_SignalingType != typeof(MockSignaling))
293-
{
294-
LogAssert.Expect(LogType.Error, new Regex("."));
295-
}
296-
297291
signaling1.SendOffer(connectionId, m_DescOffer);
298292
yield return new WaitForSeconds(3);
299293
// Do not receive offer other signaling if not connected same sendoffer connectionId in private mode

0 commit comments

Comments
 (0)