Fix race condition when preparing upload folder

[danxuliu]

Before any upload is submitted the upload is registered in a list of known uploads; this is needed to retrieve the upload object at several points of the upload process. When a chunked upload is submitted first a directory to upload all the chunks is created and, once that is done, the chunks are sent; in order to send a chunk the upload object needs to be retrieved from the list of known uploads.

When all the active uploads were finished the list of known uploads was cleared. However, an upload is not active until it actually starts sending the data, so while waiting for the upload directory to be created the upload is already in the list of known uploads yet not active. Due to all this, if the active uploads finished while another pending upload was waiting for the upload directory to be created that pending upload would be removed from the list of known uploads too, and once the directory was created and thus the chunks were sent a field of a null upload object would be accessed thus causing a failure.

Instead of removing all the known uploads at once when the active uploads finish now each upload is explicitly removed when it finishes.

How to test
Being a race condition sometimes this happens and sometimes it does not; it is necessary to add a sleep(10) to createDirectory in DAV to be able to test this consistently. The instructions below assume that this hack was added.

• Open the Files app
• Upload a file larger than 10MiB (so chunked uploads are used)
• Upload another file larger than 10MiB before the first one has finished (there is no need to wait for the first upload to actually start)

Expected results
The files are uploaded successfully (note that the progress bar is shown once the uploads actually start, so it will not be shown while waiting for the upload directory to be created).

Actual result
The first file is uploaded successfully; the second is not, and its progress bar is never hidden.

@nextcloud/javascript

[skjnldsv]

I love your explanation posts! :D
Code makes sense indeed. Let's wait for ci

[danxuliu]

Argh, there is yet another race condition in the spaghetti that is the JavaScript upload code when canceling chunked uploads :'( [Insert here title of Michael Ende's famous novel]

[MorrisJobke]

Tested and works 👍 Let's get this in for now and fix the other issue in a separate PR.

[skjnldsv]

Ci is crying, @danxuliu can you take a look?

[codecov]

Codecov Report

Merging #9245 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #9245 +/- ##
==========================================
- Coverage 51.9% 51.8% -0.1% - Complexity 25365 25393 +28 
==========================================
Files 1607 1607 Lines 95330 95516 +186 Branches 1394 1394 ==========================================
+ Hits 49482 49486 +4 - Misses 45848 46030 +182

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Comments/Manager.php	63.72% <0%> (-32.13%)	107% <0%> (+27%)
core/js/js.js	65.41% <0%> (-0.56%)	0% <0%> (ø)
lib/private/DB/ConnectionFactory.php	67.07% <0%> (+8.09%)	24% <0%> (+1%)	⬆️

[danxuliu]

@skjnldsv

Ci is crying, @danxuliu can you take a look?

Just for the record, it was unrelated ;-)

[MorrisJobke]

@danxuliu Mind to do the backport? 🏓 😉

[danxuliu]

Backport is in #9454 ;-)