Crash when fetching ParseFile for the second time

vzukanov commented 2 years ago

New Issue Checklist

[x] I am not disclosing a vulnerability.
[x] I am not just asking a question.
[x] I have searched through existing issues.
[x] I can reproduce the issue with the latest version of Parse Server and the Parse Android SDK.

Issue Description

I have ParseObject that stores a file (~30MB PDF). When I attempt to get the file in Android, everything works as expected. This is the approaximate flow (executed on a background thread):

ParseFile parseFile = rootObject.getParseFile("file");
File file = parseFile.getFile();

However, in some situations, the same flow can be triggered for the second time (from different entry point) before Parse Android SDK managed to download the contents of the file from the server. In that case, the following RuntimeException is thrown.

Looks like Android SDK doesn’t handle this scenario of concurrent fetching of the same file from multiple threads. I see this issue from 2016 which reported basically the same problem.

Steps to reproduce

Fetch the same ParseFile from multiple threads

Actual Outcome

Exception thrown

Expected Outcome

SDK "merges" all concurrent requests for the same file into one download and cache flow and notifies all waiting threads when the file is available

Environment

Parse Android SDK

SDK version: 3.0.0
Operating system version: Android 11

Server

Parse Server version: 4.10.4
Operating system: Ubuntu 20.04
Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): DigitalOcean

Database

System (MongoDB or Postgres): MongoDb

Logs

03-08 14:45:27.752 11591 11987 E AndroidRuntime: FATAL EXCEPTION: pool-5-thread-4
03-08 14:45:27.752 11591 11987 E AndroidRuntime: Process: com.myapp.android, PID: 11591
03-08 14:45:27.752 11591 11987 E AndroidRuntime: java.lang.RuntimeException: java.io.FileNotFoundException: Source '/data/user/0/com.myapp.android/cache/com.parse/files/https:/myserver.com/parse/files/myappid/67dfb4b64a2682765d9b34f34e5bfe00_s-14.pdf.tmp' does not exist
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.ParseTaskUtils.wait(ParseTaskUtils.java:40)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.ParseFile.getFile(ParseFile.java:453)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.myapp.data.FetchDataSync.fetchDataSync(FetchDataSync.java:44)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.myapp.data.FetchDataFlow.lambda$fetchDataAndNotify$0$FetchDataFlow(FetchDataFlow.java:53)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.myapp.data.-$$Lambda$FetchDataFlow$tR5BMEB9OXkont1M9Cyfju24BY8.run(Unknown Source:6)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at java.lang.Thread.run(Thread.java:923)
03-08 14:45:27.752 11591 11987 E AndroidRuntime: Caused by: java.io.FileNotFoundException: Source '/data/user/0/com.myapp.android/cache/com.parse/files/https:/myserver.app/parse/files/myappid/67dfb4b64a2682765d9b34f34e5bfe00_shvilim-14.pdf.tmp' does not exist
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.ParseFileUtils.moveFile(ParseFileUtils.java:176)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.ParseFileController.lambda$fetchAsync$2(ParseFileController.java:225)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.-$$Lambda$ParseFileController$lYrLoAmtAOSJPVdiXWr93eIKenk.then(Unknown Source:6)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.boltsinternal.Task.lambda$completeAfterTask$9(Task.java:500)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    at com.parse.boltsinternal.-$$Lambda$Task$in9EOMtNYir2i-wDxNv-3vGMwIs.run(Unknown Source:8)
03-08 14:45:27.752 11591 11987 E AndroidRuntime:    ... 3 more

parse-github-assistant[bot] commented 2 years ago

Thanks for opening this issue!

🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

mtrezza commented 2 years ago

I wonder whether we can add a test case to demo this?

I noticed that the file path looks strange:

/data/user/0/com.myapp.android/cache/com.parse/files/https:/myserver.com/parse/files/myappid/67dfb4b64a2682765d9b34f34e5bfe00_s-14.pdf.tmp

Is a directory name even allowed to contain a colon :?

mtrezza commented 1 year ago

@vzukanov Any updates on this?

vzukanov commented 1 year ago

@mtrezza , not sure what kind of update I can give. This looks like a concurrency bug somewhere inside the lib. We worked around this problem by keeping track of the files we're downloading during sync and preventing concurrent calls to ParseFile.getData() method. This is the class that we use:


/**
 * This global object should be used during sync with the server to track which ParseFile's need
 * to be downloaded. It's important to reset this object's state before each sync.
 */
public class ParseFilesDownloadTracker {

    private final Object LOCK = new Object();

    // this list is required to work around this bug in Parse SDK:
    // https://github.com/parse-community/Parse-SDK-Android/issues/1155
    private final List<String> encounteredFileNames = new ArrayList<>();

    public boolean shouldDownloadParseFileData(ParseFile parseFile) {
        synchronized (LOCK) {
            if (parseFile.isDataAvailable()) {
                return false; // data is in local datastore
            } else if (encounteredFileNames.contains(parseFile.getName())) {
                return false; // download of this file has already been initiated
            } else {
                encounteredFileNames.add(parseFile.getName());
                return true;
            }
        }
    }

    public void reset() {
        synchronized (LOCK) {
            encounteredFileNames.clear();
        }
    }
}

techyourchance commented 1 year ago

@mtrezza , I decided to try and fix this bug myself. Please review the linked PR and let me know what you think (this is another GitHub account of vzukanov).

rommansabbir commented 1 year ago

PR Merged: #1180

parse-community / Parse-SDK-Android