Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrency issue leading to corrupted uploads #8

Open
rzajac opened this issue Nov 10, 2014 · 7 comments
Open

Concurrency issue leading to corrupted uploads #8

rzajac opened this issue Nov 10, 2014 · 7 comments

Comments

@rzajac
Copy link

rzajac commented Nov 10, 2014

I know the title is bit vague so let me explain.

During my tests I wanted to simulate big file uploads with relatively small files so on the front end I set:

simultaneousUploads: 3 (which is I believe default setting)
`chunkSize: 2048 (which is 1024*1024 by default)

This gave me kind of simulation of multiple chunk uploads. Using the library on the back end you have to follow four steps:

  1. validateChunk()
  2. saveChunk()
  3. validateFile()
  4. save()

If you have multiple concurrent uploads this may lead to corrupted files. Lets take a case of last two chunks being uploaded where x axis is time:

chunk1:  |---validateChunk---||---saveChunk---||---validateFile---||---save---|
chunk2:               |---validateChunk---||---saveChunk---||---validateFile---|

when validateFile is called on chunk1 the saveChunk for chunk2 already begun so chunk1::validateFile will return true and proceed to save file with chunk2 not fully saved.

I saw this error in my logs already few times. The above example will also lead to double save call. And in my case it leads to duplicate key error in the database.

To fix the problem library would have to implement locking not only in save but in validateFile methods . Its not enough to call file_exists in validateFile. We have to know the chunk is fully uploaded.

@AidasK
Copy link
Member

AidasK commented Nov 10, 2014

https://github.com/flowjs/flow-php-server/blob/master/src/Flow/File.php#L90
Are you sure?
File is not going to be valid if chunk is not fully copied, size won't match.

Save method won't trigger twice because of https://github.com/flowjs/flow-php-server/blob/master/src/Flow/File.php#L104
fh = fopen($destination, 'wb'); is successful only then file is not present. In other words, it should not be created earlier for this to pass.

Also note, that this library is not made for Windows, php flock implementation differs for it.

@AidasK
Copy link
Member

AidasK commented Nov 10, 2014

To be more specific, fopen can't be successful because other process will have an open lock for it.
https://github.com/flowjs/flow-php-server/blob/master/src/Flow/File.php#L108

Ofcourse, I might be wrong, but you can check it.

@rzajac
Copy link
Author

rzajac commented Nov 10, 2014

I have tested it like I described. And I saw errors in my logs. I have not spent much time on it yet.

For now changing to simultaneousUploads: 1 solved the problem for me. I also have to check if this is not something specific to Phalcon PHP framework.

Will get more info when I do more tests and debugging.

@rzajac rzajac mentioned this issue Nov 15, 2014
@baopham
Copy link

baopham commented Mar 15, 2015

I tried simultaneousUploads: 1, but it just stops there and doesn't upload the rest of the chunks.

@dkarter
Copy link

dkarter commented Jul 21, 2015

I have this issue as well: simultaneous pdf uploads appear to work but the files arrive corrupted, sometimes it works and sometimes doesn't. I am using Ruby on Rails as my server.

@AidasK
Copy link
Member

AidasK commented Jul 22, 2015

Could you share your ruby on rails file handling code? Does it do locking?

@enfrte
Copy link

enfrte commented Jun 21, 2022

I has a similar issue with some uploads not arriving, but reported as uploaded. I was getting errors like Unable to clear session lock record... and Failed to read session data: memcached... thrown at the same time.

Setting simultaneousUploads: from 3 to 1 solved the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants