Three.js: FileLoader with ArrayBuffer high loading frequency leads to DOMException

[kaisalmen]

Description of the problem

12458 "Fix FileLoader duplicate request issues" seems to have introduced a new problem. When the same file is loaded with high frequency and ArrayBuffers are used then this message is likely to appear:

Uncaught DOMException ... ArrayBuffer at index 0 is already neutered.

You can reproduce this easily with example webgl_loader_obj2_run_director. Select a high worker count; minimum 4 better 8 or 12. Watch the log for the appearance of the error. Due to the DOMException the worker feed gets stalled.

When I revert to r87 the issue is gone. I have a potential fix, but I am not 100% sure why it works:
https://github.com/kaisalmen/three.js/blob/FileLoaderRegression/src/loaders/FileLoader.js

I made loading a member of FileLoader and I think due to the now existing dependency memory of the two objects is no longer released independently. @takahirox do you mind have a look?

Kai

Three.js version

• [x] Dev
• [x] r89
• [x] r88

Browser

• [x] All of them

OS

• [x] All of them

Hardware Requirements (graphics card, VR Device, ...)

[takahirox]

I will look into.

@mrdoob Should we fix by r90? Or r91?

[mrdoob]

r91 for sure.

[kaisalmen]

@takahirox This can easily be reproduced by using the same loader with the same file directly one after the other (as done in linked PR) or for example in a loop.

[takahirox]

Probably this's caused by attempting to access invalidated transferable object (ArrayBuffer in this case).

As you know, transferable object is no longer accessible in main thread once it's transferred to a worker (unless it's transferred back from worker to main thread). "Transfer" is kinda "move", not "copy".

If main thread attempts to access invalidated transferable object you'll see the error mentioned above.

I haven't read that example, OBJLoader2, and LoaderSupport yet but I guess the example processes like this

var url = "path_to_file";
var loader = new THREE.FileLoader();
loader.load( url, function ( buffer ) {
    transfer_to_worker_a( buffer );
} );
loader.load( url, function ( buffer ) {
    transfer_to_worker_b( buffer );  // causing an error
} );

New FileLoader calls callback function with the same object for duplicate URL request. So second transfer in the code above would cause an error. An easy workaround would be transfer the copy of ArrayBuffer to worker.

I'm gonna think how to solve the root issue the other day. (Sorry time to sleep here now!)

[kaisalmen]

@takahirox thanks for the analysis. Neutered ArrayBuffer have byteLenght zero. This seems to be the only way to detect that the "ownership" was transferred. There seems to be no explicit check functions available.
I should copy the ArrayBuffers in my example if I know beforehand that I want to re-use them.

Independently FileLoader could invalidate its cache and force download of a file again if the byteLength is unexpectedly zero.

Btw, we are on the same time-zone now. 😄

[takahirox]

Independently FileLoader could invalidate its cache and force download of a file again if the byteLength is unexpectedly zero.

IMO this'd make the situation a bit too complex. I'd like to separate FileLoader from the side effect of worker use for the simplicity.

And two thoughts.

First, this neutering behavior is well known so it'd better that workers manager (LoaderSupport?) manages neutered objects regardless FileLoader returns the same object, (I assume workers manager module is separated well from loader and loading manager.)

Second, a bit off topic tho, new FileLoader returns the same object to duplicate requests even in THREE.Cache.enabled = false case, this'd be kinda confusing. In this case, duplicate requests means two or more requests with the same URL sent before the loading for the first request is completed.

Having three cache levels or having only 1. and 3. would be more straightforward.

1. always returns the same object to the same URL request (Cache.enabled = true)
2. generally returns new object but returns the same object to duplicate requests (Cache.enabled = false)
3. always returns new object (new)

@mrdoob What do you think? I prefer having only 1. and 3. because it's clear to users.

Oh, cool. Both we're in Germany now!

[kaisalmen]

First, this neutering behavior is well known so it'd better that workers manager (LoaderSupport?) manages neutered objects regardless FileLoader returns the same object, (I assume workers manager module is separated well from loader and loading manager.)

Yes, I can make the WorkerSupport detect it and cancel appropriately for example. I will take care of that.

I prefer having only 1. and 3. because it's clear to users.

I agree. This is clear. Still, an issue is that the user invalidates the cache, just by intentional or unintentional usage of the returned ArrayBuffer and the FileLoaderdoes not recognize it. Shouldn't in case 1 FileLoader at least check the ownership of the content and log a warning if it is gone. Otherwise, you may have a similar issue coming back in the future.

[takahirox]

I prefer just specifying "object loaded by FileLoader is readonly (if Cache is enable)" because we can keep FileLoader simple. I think other modules or user code can manage the objects out of FileLoader, for example passing the copy of original objects for update purpose.

[mrdoob]

@mrdoob What do you think? I prefer having only 1. and 3. because it's clear to users.

I'm struggling to follow this one... But I like the idea of not over-complicating the loader because of workers...

[kaisalmen]

@takahirox Sounds good to me

[kaisalmen]

@takahirox @mrdoob I have a fix available for WorkerSupport / WorkerDirector. You can now set boolean forceWorkerDataCopy. It will copy the ArrayBuffer before it is sent off to the Worker. I made this switchable as it is not always necessary. Nice result of the code review and fix for WorkerSupport / WorkerDirector is that the parallels demo is now able to use 100% CPU (all eight logical cores on my box 👍 )

Larger PR for this problem, OBJLoader2 Parser optimization and LoaderSupport clean-up will come later this week. Need to update examples and documentation.

Edit: PR is here now: https://github.com/mrdoob/three.js/pull/13524