Merge pull request #396 from koush/race

Fix race conditions by switching to node webcrypto
shinyoshiaki · Aug 7, 2024 · 47b12fa · 47b12fa
2 parents 528966e + a583b70
commit 47b12fa
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 38 deletions.
diff --git a/packages/ice/src/dns/lookup.ts b/packages/ice/src/dns/lookup.ts
@@ -15,6 +15,10 @@ export class MdnsLookup {
   cache = new Map<string, Promise<string>>();
   mdnsInstance = mdns();
 
+  constructor() {
+    this.mdnsInstance.setMaxListeners(50);
+  }
+
   lookup(host: string): Promise<string> {
     return new Promise((r, f) => {
       const cleanup = () => {

diff --git a/packages/ice/src/ice.ts b/packages/ice/src/ice.ts
@@ -157,6 +157,7 @@ export class Connection {
           "host",
         );
 
+        this.pairLocalProtocol(protocol)
         candidates.push(protocol.localCandidate);
         if (cb) {
           cb(protocol.localCandidate);
@@ -911,22 +912,32 @@ export class Connection {
     }
   }
 
-  private pairRemoteCandidate = (remoteCandidate: Candidate) => {
-    for (const protocol of this.protocols) {
+  private tryPair(protocol: Protocol, remoteCandidate: Candidate) {
+    if (
+      protocol.localCandidate?.canPairWith(remoteCandidate) &&
+      !this.findPair(protocol, remoteCandidate)
+    ) {
+      const pair = new CandidatePair(protocol, remoteCandidate);
       if (
-        protocol.localCandidate?.canPairWith(remoteCandidate) &&
-        !this.findPair(protocol, remoteCandidate)
+        this.options.filterCandidatePair &&
+        !this.options.filterCandidatePair(pair)
       ) {
-        const pair = new CandidatePair(protocol, remoteCandidate);
-        if (
-          this.options.filterCandidatePair &&
-          !this.options.filterCandidatePair(pair)
-        ) {
-          continue;
-        }
-        this.checkList.push(pair);
-        this.setPairState(pair, CandidatePairState.WAITING);
+        return;
       }
+      this.checkList.push(pair);
+      this.setPairState(pair, CandidatePairState.WAITING);
+    }
+  }
+
+  private pairLocalProtocol(protocol: Protocol) {
+    for (const remoteCandidate of this.remoteCandidates) {
+      this.tryPair(protocol, remoteCandidate);
+    }
+  }
+
+  private pairRemoteCandidate = (remoteCandidate: Candidate) => {
+    for (const protocol of this.protocols) {
+      this.tryPair(protocol, remoteCandidate);
     }
   };
 

diff --git a/packages/webrtc/src/peerConnection.ts b/packages/webrtc/src/peerConnection.ts
@@ -508,8 +508,10 @@ export class RTCPeerConnection extends EventTarget {
     });
 
     iceTransport.iceGather.onIceCandidate = (candidate) => {
-      if (!this.localDescription) return;
-
+      if (!this.localDescription) {
+        log("localDescription not found when ice candidate was gathered");
+        return;
+      }
       if (this.config.bundlePolicy === "max-bundle" || this.remoteIsBundled) {
         candidate.sdpMLineIndex = 0;
         const media = this._localDescription?.media[0];
@@ -653,14 +655,6 @@ export class RTCPeerConnection extends EventTarget {
     // for trickle ice
     this.setLocal(description);
 
-    // connect transports
-    if (description.type === "answer") {
-      this.connect().catch((err) => {
-        log("connect failed", err);
-        this.setConnectionState("failed");
-      });
-    }
-
     // # gather candidates
     const connected = this.iceTransports.find(
       (transport) => transport.state === "connected",
@@ -676,6 +670,14 @@ export class RTCPeerConnection extends EventTarget {
       );
     }
 
+    // connect transports
+    if (description.type === "answer") {
+      this.connect().catch((err) => {
+        log("connect failed", err);
+        this.setConnectionState("failed");
+      });
+    }
+
     description.media
       .filter((m) => ["audio", "video"].includes(m.kind))
       .forEach((m, i) => {
@@ -998,18 +1000,6 @@ export class RTCPeerConnection extends EventTarget {
       });
     }
 
-    const connected = this.iceTransports.find(
-      (transport) => transport.state === "connected",
-    );
-    if (this.remoteIsBundled && connected) {
-      // no need to gather ice candidates on an existing bundled connection
-      await connected.iceGather.gather();
-    } else {
-      await Promise.allSettled(
-        transports.map((iceTransport) => iceTransport.iceGather.gather()),
-      );
-    }
-
     this.negotiationneeded = false;
     if (this.shouldNegotiationneeded) {
       this.needNegotiation();

diff --git a/packages/webrtc/src/transport/dtls.ts b/packages/webrtc/src/transport/dtls.ts
@@ -52,6 +52,7 @@ export class RTCDtlsTransport {
   readonly onStateChange = new Event<[DtlsState]>();
 
   localCertificate?: RTCCertificate;
+  localCertificatePromise?: Promise<RTCCertificate>;
   private remoteParameters?: RTCDtlsParameters;
 
   constructor(
@@ -72,7 +73,15 @@ export class RTCDtlsTransport {
   }
 
   async setupCertificate() {
-    if (!this.localCertificate) {
+    if (this.localCertificate) {
+      return this.localCertificate;
+    }
+
+    if (this.localCertificatePromise) {
+      return this.localCertificatePromise;
+    }
+
+    this.localCertificatePromise = (async () => {
       const { certPem, keyPem, signatureHash } =
         await CipherContext.createSelfSignedCertificateWithKey(
           {
@@ -86,8 +95,10 @@ export class RTCDtlsTransport {
         certPem,
         signatureHash,
       );
-    }
-    return this.localCertificate;
+      return this.localCertificate;
+    })();
+
+    return this.localCertificatePromise;
   }
 
   setRemoteParams(remoteParameters: RTCDtlsParameters) {