Use HandleAllocator for fetch API. NFC (#19337)

Author: sbc100

src/Fetch.js

@@ -5,14 +5,16 @@
  */
 
 var Fetch = {
-  // Map of integer fetch id to XHR request object
-  xhrs: {},
+  // HandleAllocator for XHR request object
+  // xhrs: undefined,
 
-  // The web worker that runs proxied file I/O requests. (this field is populated on demand, start as undefined to save code size)
+  // The web worker that runs proxied file I/O requests. (this field is
+  // populated on demand, start as undefined to save code size)
   // worker: undefined,
 
   // Specifies an instance to the IndexedDB database. The database is opened
-  // as a preload step before the Emscripten application starts. (this field is populated on demand, start as undefined to save code size)
+  // as a preload step before the Emscripten application starts. (this field is
+  // populated on demand, start as undefined to save code size)
   // dbInstance: undefined,
 
 #if FETCH_SUPPORT_INDEXEDDB
@@ -40,8 +42,12 @@ var Fetch = {
   },
 #endif
 
-  staticInit: function() {
+  init: function() {
+    Fetch.xhrs = new HandleAllocator();
 #if FETCH_SUPPORT_INDEXEDDB
+#if PTHREADS
+    if (ENVIRONMENT_IS_PTHREAD) return;
+#endif
     var onsuccess = (db) => {
 #if FETCH_DEBUG
       dbg('fetch: IndexedDB successfully opened.');
@@ -293,8 +299,12 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
       xhr.setRequestHeader(keyStr, valueStr);
     }
   }
-  var id = {{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.id, 'u32') }}};
-  Fetch.xhrs[id] = xhr;
+
+  var id = Fetch.xhrs.allocate(xhr);
+#if FETCH_DEBUG
+  dbg('fetch: id=' + id);
+#endif
+  {{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.id, 'id', 'u32') }}};
   var data = (dataPtr && dataLength) ? HEAPU8.slice(dataPtr, dataPtr + dataLength) : null;
   // TODO: Support specifying custom headers to the request.
 
@@ -334,7 +344,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
 
   xhr.onload = (e) => {
     // check if xhr was aborted by user and don't try to call back
-    if (!(id in Fetch.xhrs)) { 
+    if (!Fetch.xhrs.has(id)) {
       return;
     }
     saveResponseAndStatus();
@@ -352,7 +362,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
   };
   xhr.onerror = (e) => {
     // check if xhr was aborted by user and don't try to call back
-    if (!(id in Fetch.xhrs)) { 
+    if (!Fetch.xhrs.has(id)) {
       return;
     }
 #if FETCH_DEBUG
@@ -363,7 +373,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
   };
   xhr.ontimeout = (e) => {
     // check if xhr was aborted by user and don't try to call back
-    if (!(id in Fetch.xhrs)) { 
+    if (!Fetch.xhrs.has(id)) {
       return;
     }
 #if FETCH_DEBUG
@@ -373,7 +383,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
   };
   xhr.onprogress = (e) => {
     // check if xhr was aborted by user and don't try to call back
-    if (!(id in Fetch.xhrs)) { 
+    if (!Fetch.xhrs.has(id)) {
       return;
     }
     var ptrLen = (fetchAttrLoadToMemory && fetchAttrStreamData && xhr.response) ? xhr.response.byteLength : 0;
@@ -405,7 +415,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
   };
   xhr.onreadystatechange = (e) => {
     // check if xhr was aborted by user and don't try to call back
-    if (!(id in Fetch.xhrs)) {
+    if (!Fetch.xhrs.has(id)) {
       {{{ runtimeKeepalivePop() }}}
       return;
     }
@@ -560,27 +570,27 @@ function startFetch(fetch, successcb, errorcb, progresscb, readystatechangecb) {
 }
 
 function fetchGetResponseHeadersLength(id) {
-    return lengthBytesUTF8(Fetch.xhrs[id].getAllResponseHeaders()) + 1;
+  return lengthBytesUTF8(Fetch.xhrs.get(id).getAllResponseHeaders()) + 1;
 }
 
 function fetchGetResponseHeaders(id, dst, dstSizeBytes) {
-    var responseHeaders = Fetch.xhrs[id].getAllResponseHeaders();
-    var lengthBytes = lengthBytesUTF8(responseHeaders) + 1;
-    stringToUTF8(responseHeaders, dst, dstSizeBytes);
-    return Math.min(lengthBytes, dstSizeBytes);
+  var responseHeaders = Fetch.xhrs.get(id).getAllResponseHeaders();
+  var lengthBytes = lengthBytesUTF8(responseHeaders) + 1;
+  stringToUTF8(responseHeaders, dst, dstSizeBytes);
+  return Math.min(lengthBytes, dstSizeBytes);
 }
 
 //Delete the xhr JS object, allowing it to be garbage collected.
 function fetchFree(id) {
 #if FETCH_DEBUG
-    dbg("fetch: Deleting id:" + id + " of " + Fetch.xhrs);
+  dbg("fetch: fetchFree id:" + id);
 #endif
-    var xhr = Fetch.xhrs[id];
-    if (xhr) {
-        delete Fetch.xhrs[id];
-        // check if fetch is still in progress and should be aborted
-        if (xhr.readyState > 0 && xhr.readyState < 4) {
-            xhr.abort();
-        }
+  if (Fetch.xhrs.has(id)) {
+    var xhr = Fetch.xhrs.get(id);
+    Fetch.xhrs.free(id);
+    // check if fetch is still in progress and should be aborted
+    if (xhr.readyState > 0 && xhr.readyState < 4) {
+      xhr.abort();
     }
+  }
 }

src/library.js

@@ -3667,6 +3667,9 @@ mergeInto(LibraryManager.library, {
 #endif
       return this.allocated[id];
     };
+    this.has = function(id) {
+      return this.allocated[id] !== undefined;
+    };
     this.allocate = function(handle) {
       var id = this.freelist.pop() || this.allocated.length;
       this.allocated[id] = handle;

src/library_fetch.js

@@ -7,11 +7,8 @@
 #include Fetch.js
 
 var LibraryFetch = {
-#if PTHREADS
-  $Fetch__postset: 'if (!ENVIRONMENT_IS_PTHREAD) Fetch.staticInit();',
-#else
-  $Fetch__postset: 'Fetch.staticInit();',
-#endif
+  $Fetch__postset: 'Fetch.init();',
+  $Fetch__deps: ['$HandleAllocator'],
   $Fetch: Fetch,
   _emscripten_fetch_get_response_headers_length__deps: ['$lengthBytesUTF8'],
   _emscripten_fetch_get_response_headers_length: fetchGetResponseHeadersLength,

system/lib/fetch/emscripten_fetch.c

@@ -67,12 +67,10 @@ void emscripten_fetch_attr_init(emscripten_fetch_attr_t* fetch_attr) {
   memset(fetch_attr, 0, sizeof(emscripten_fetch_attr_t));
 }
 
-static int globalFetchIdCounter = 1;
 emscripten_fetch_t* emscripten_fetch(emscripten_fetch_attr_t* fetch_attr, const char* url) {
-  if (!fetch_attr)
-    return 0;
-  if (!url)
-    return 0;
+  if (!fetch_attr || !url) {
+    return NULL;
+  }
 
   const bool synchronous = (fetch_attr->attributes & EMSCRIPTEN_FETCH_SYNCHRONOUS) != 0;
   const bool readFromIndexedDB =
@@ -85,14 +83,13 @@ emscripten_fetch_t* emscripten_fetch(emscripten_fetch_attr_t* fetch_attr, const
 #ifdef FETCH_DEBUG
     emscripten_console_errorf("emscripten_fetch('%s') failed! Synchronous blocking XHRs and IndexedDB operations are not supported on the main browser thread. Try dropping the EMSCRIPTEN_FETCH_SYNCHRONOUS flag, or run with the linker flag --proxy-to-worker to decouple main C runtime thread from the main browser thread.", url);
 #endif
-    return 0;
+    return NULL;
   }
 
-  emscripten_fetch_t* fetch = (emscripten_fetch_t*)malloc(sizeof(emscripten_fetch_t));
-  if (!fetch)
-    return 0;
-  memset(fetch, 0, sizeof(emscripten_fetch_t));
-  fetch->id = globalFetchIdCounter++; // TODO: make this thread-safe!
+  emscripten_fetch_t* fetch = (emscripten_fetch_t*)calloc(1, sizeof(emscripten_fetch_t));
+  if (!fetch) {
+    return NULL;
+  }
   fetch->userData = fetch_attr->userData;
   fetch->__attributes.timeoutMSecs = fetch_attr->timeoutMSecs;
   fetch->__attributes.attributes = fetch_attr->attributes;
@@ -117,61 +114,35 @@ emscripten_fetch_t* emscripten_fetch(emscripten_fetch_attr_t* fetch_attr, const
   STRDUP_OR_ABORT(fetch->__attributes.userName, fetch_attr->userName);
   STRDUP_OR_ABORT(fetch->__attributes.password, fetch_attr->password);
   STRDUP_OR_ABORT(fetch->__attributes.overriddenMimeType, fetch_attr->overriddenMimeType);
+#undef STRDUP_OR_ABORT
+
   if (fetch_attr->requestHeaders) {
     size_t headersCount = 0;
     while (fetch_attr->requestHeaders[headersCount])
       ++headersCount;
     const char** headers = (const char**)malloc((headersCount + 1) * sizeof(const char*));
     if (!headers) {
       fetch_free(fetch);
-      return 0;
+      return NULL;
     }
     memset((void*)headers, 0, (headersCount + 1) * sizeof(const char*));
 
     for (size_t i = 0; i < headersCount; ++i) {
       headers[i] = strdup(fetch_attr->requestHeaders[i]);
-      if (!headers[i])
-
-      {
+      if (!headers[i]) {
         for (size_t j = 0; j < i; ++j) {
           free((void*)headers[j]);
         }
         free((void*)headers);
         fetch_free(fetch);
-        return 0;
+        return NULL;
       }
     }
     headers[headersCount] = 0;
     fetch->__attributes.requestHeaders = headers;
   }
 
-#undef STRDUP_OR_ABORT
-
-// In asm.js we can use a fetch worker, which is created from the main asm.js
-// code. That lets us do sync operations by blocking on the worker etc.
-// In the wasm backend we don't have a fetch worker implemented yet, however,
-// we can still do basic synchronous fetches in the same places: if we can
-// block on another thread then we aren't the main thread, and if we aren't
-// the main thread then synchronous xhrs are legitimate.
-#if __EMSCRIPTEN_PTHREADS__ && !defined(__wasm__)
-  const bool waitable = (fetch_attr->attributes & EMSCRIPTEN_FETCH_WAITABLE) != 0;
-  // Depending on the type of fetch, we can either perform it in the same Worker/thread than the
-  // caller, or we might need to run it in a separate Worker. There is a dedicated fetch worker that
-  // is available for the fetch, but in some scenarios it might be desirable to run in the same
-  // Worker as the caller, so deduce here whether to run the fetch in this thread, or if we need to
-  // use the fetch-worker instead.
-  if (waitable // Waitable fetches can be synchronously waited on, so must always be proxied
-      || (synchronous &&
-           (readFromIndexedDB || writeToIndexedDB))) // Synchronous IndexedDB access needs proxying
-  {
-    fetch->__proxyState = 1; // sent to proxy worker.
-    emscripten_proxy_fetch(fetch);
-
-    if (synchronous)
-      emscripten_fetch_wait(fetch, INFINITY);
-  } else
-#endif
-    emscripten_start_fetch(fetch);
+  emscripten_start_fetch(fetch);
   return fetch;
 }
 

test/fetch/xhr_abort.cpp

@@ -18,7 +18,7 @@ void handleError(emscripten_fetch_t *fetch) {
   bool isAbortedStatus = fetch->status == (unsigned short) -1;
   assert(isAbortedStatus); // should have aborted status
   EM_ASM({
-    const xhr = Fetch.xhrs[$0];
+    const xhr = Fetch.xhrs.get($0);
     const oldReadyStateChangeHandler = xhr.onreadystatechange;
     // Overriding xhr handlers to check if xhr.abort() was called
     xhr.onreadystatechange = (e) => {