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

multi threads DbStoragePlainFile.createPaperDir() will throw new RuntimeException("Couldn't create Paper dir: " + mDbPath); #114

Closed
bigkun opened this issue Jan 24, 2018 · 8 comments

Comments

@bigkun
Copy link

bigkun commented Jan 24, 2018

multi threads DbStoragePlainFile.createPaperDir() will throw new RuntimeException("Couldn't create Paper dir: " + mDbPath);
because “boolean isReady = new File(mDbPath).mkdirs();” will return false.

fix:

 private void createPaperDir() {
        synchronized (DbStoragePlainFile.class) {
            if (!new File(mDbPath).exists()) {
                boolean isReady = new File(mDbPath).mkdirs();
                if (!isReady) {
                    throw new RuntimeException("Couldn't create Paper dir: " + mDbPath);
                }
            }
        }
    }
@pilgr
Copy link
Owner

pilgr commented Jan 25, 2018

Thanks, now I see where the issue may happen. Did you see the issue it in your app?
One moment is that createPaperDir should be synchronized by current DbStoragePlainFile instance, not by DbStoragePlainFile class.

@jeff-huston
Copy link

I believe a race condition in our app ran into this.

We had a first operation that would complete, and then it would kick off a "save" on another thread (this "save" would destroy() and then insert() into Paper).

A second operation would run immediately after the first operation and try to read by calling getAllKeys() in Paper.

getAllKeys() appears to surprisingly make a call to createPaperDir() within assertInit(). Within this createPaperDir() we were seeing this RuntimeException, which we suspect was occurring when the aforementioned functions would happen to get called in this order:

  1. destroy() from the first operation
  2. if (!new File(mDbPath).exists()) from within createPaperDir() in the second operation
  3. insert() from the first operation
  4. new File(mDbPath).mkdirs() from within createPaperDir() in the second operation

Clearly the race condition we created in our app could have caused different, even harder-to-diagnose problems, even if this reported Issue didn't exist. Just wanted to share our experience.

@svenoaks
Copy link

I see this crash happening in the beta version of my app with only a couple thousand users. I think "Reading/writing for different keys can be done in parallel" is not really safe at the moment.

@wassil
Copy link

wassil commented Jan 14, 2020

Happens to our SDK as well. The SDK is used to report analytic events, so it's definitely possible for 2 threads to use the database at the same time.
I'm also able to reproduce this easily in unit tests, just spawn 10 threads in for loop and use database in another for loop 10 times.

@wassil
Copy link

wassil commented Jan 14, 2020

@pilgr seems like bigkun suggested a correct fix 2 years ago, is this getting fixed?
Should I do it?

@pilgr
Copy link
Owner

pilgr commented Feb 17, 2020

@wassil thanks but I'm already working on the fix, please review PR once it's ready

@pilgr
Copy link
Owner

pilgr commented Feb 29, 2020

Please review the PR with the fix #168

@pilgr
Copy link
Owner

pilgr commented May 11, 2020

Fixed and merged to master, included in the new version 2.7.1

@pilgr pilgr closed this as completed May 11, 2020
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