realm / realm-java

Realm is a mobile database: a replacement for SQLite & ORMs
http://realm.io
Apache License 2.0
11.46k stars 1.75k forks source link

Add deleteRealm(configuration, force) #1586

Open renannery opened 9 years ago

renannery commented 9 years ago

Hi there!

First of all I'd like to thank you about the amazing job that you guys are doing with Realm and contributing with the Android developers community :+1:

Here in our team we are trying to solve a situation with Thread doing a Retrofit request and afterwards save data on Realm database.

The issue steps:

1) There is a thread doing some hard work with Retrofit request and realm persistence inside a Runnable object

2) In the meantime the user hit a button that will delete realm file (It is a logoff button)

3) As you can see, the first thread still doing some hard work with a open realm instance. At this point we are not able to delete realm file because there is an open realm instance inside the thread.

We have tried to interrupt the thread and implement some logic to deal with InterruptException but sounds like a code smells.

Another approach was a Thread.sleep inside a try catch in realm delete method that will wait for 5 seconds (time limit for retrofit http requests).

My question is, is there a way to close realm instance in safely mode inside a Thread doing some hard work?

This is our thread

public void getLocalInterests(final int localId) {
        new Thread(new Runnable() {
            Realm realm = null;

            @Override
            public void run() {

                try {

                    realm = Realm.getDefaultInstance();
                    realm.beginTransaction();

                    RealmList<Interest> interests = new API().service().getLocalInterests(localId);
                    Local local = realm.where(Local.class).equalTo(Local.FIELD_ID, localId).findFirst();
                    local.getInterests().clear();
                    local.getInterests().addAll(interests);
                    realm.copyToRealmOrUpdate(local);
                    realm.commitTransaction();

                    EventBus.getDefault().post(new GenericBus(Config.GET_LOCAL_INTERESTS));

                } catch (Exception e) {
                    EventBus.getDefault().post(new GenericBus(Config.GET_LOCAL_INTERESTS, true, e.toString()));
                    LogUtils.report("Service getLocalInterests", e, "localId: " + localId);
                } finally {
                    if (realm != null) {
                        realm.close();
                    }
                }
            }
        }).start();

And here is our logoff method

public void logoff() {
        RealmConfiguration realmConfig = realm.getConfiguration();
        realm.close();
        user = null;
        Realm.deleteRealm(realmConfig);
    }

We appreciate some help.

beeender commented 9 years ago

@renannery

For your case, I have some ideas:

  1. Create the Realm with a uuid name, and store the uuid somewhere when user login. maybe another realm, or simply SharedPreference .
  2. When user open the app, check if the uuid stored, open it or start a new login.
  3. When logoff, try to delete the Realm and if failed, just logoff (the other heavy thread is running).
  4. Find some point to clear the useless Realm (maybe when user starts the app?).

And there might be better ways if we can just interrupt the worker thread immediately and close all Realm instance. But from Realm side, it is hard to just close all the instance in every thread peacefully without creating other nasty issues since there are so many different things could happen in the worker thread.

renannery commented 9 years ago

@beeender tks for reply, I will try the 3rd and 4th options and put here the result afterwards.

beeender commented 9 years ago

@renannery Sorry for the misunderstanding i made. the 1/2/3/4 are actually the whole idea... step 1/step 2/step 3/step 4 .

renannery commented 9 years ago

Okay! I will try that.

zaki50 commented 9 years ago

@renannery how about moving Realm.getDefaultInstance(); to after the Retrofit request?

nhachicha commented 9 years ago

Hi @renannery Sounds like a classic concurrency pb you can use CountDownLatch or other Locks mechanism to do cleanup, here is an approach using a Phaser

final Phaser phaser = new Phaser(1);

// start this in a service/thread once the user logout
     public void deleteRealm (final Phaser phaser, final RealmConfiguration realmConfig) {
        phaser.arriveAndAwaitAdvance();
        // at this point all threads closed their respective instance
        Realm.deleteRealm(realmConfig);
    }

    public void doApiCall (final Phaser phaser) {
        new Thread() {
            @Override
            public void run() {
                phaser.register();
                try {
                    // do stuff here
                    TimeUnit.SECONDS.sleep(5);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                } finally {
                    phaser.arrive();
                }
            }
        }.start();
    }

doApiCall(phaser);
new Thread() {
            @Override
            public void run() {
                deleteRealm(phaser);
            }
        }.start();
kneth commented 9 years ago

@renannery Did you have a chance to try the solutions proposed by @zaki50 and @nhachicha?

renannery commented 9 years ago

Not yet guys, I will work on this topic this weekend. I will let you guys know when I've done

@zaki50 Actually I'm going in this way, I changed the way to make http request, I've updated my retrofit to version 2.0 and now I can cancel requests and I am using retrofit callback instead of synchronous call inside old Thread. By the way now I have a realm instance inside UI thread and I hope that this fix my issue, but I didn't test this aproach yet :smile:

Tks for all your help!

kneth commented 9 years ago

@renannery Don't hesitate to contact us - we might be able to help ;-)

renannery commented 9 years ago

Hey guys! Sorry the delay.

Going straight to the point, what I did:

I update my retrofit from 1.9 to 2.0, because in this way I'm able to cancel request. So, now what I do to close realm instance is create a Realm instance inside Retrofit callback, keep a reference of this retrofit object inside a static list of Retrofit calls and call cancel() when necessary to deleteRealm file, so cancelling request the method onFailure going to be call and the realm instance will be closed before realm delete.

Here is my Retrofit call method with some heavy work to be done after request

public static ArrayList<Call> calls;

....

public void getUser(final int userID, final String requesterID) {

        final Call<User> userCall = new API().service().getUser(userID);

        //static ArrayList of Retrofit Calls
        calls.add(userCall);
        userCall.enqueue(new Callback<User>() {

            //REALM INSTANCE
            Realm realm = null;

            @Override
            public void onResponse(Response<User> response, Retrofit retrofit) {
                if (response.isSuccess()) {
                    try {
                        User user = response.body();

                        realm = Realm.getDefaultInstance();
                        //Doing heavy work
                    } catch (Exception e) {
                        //Ops something wrong, do nothing....

                    } finally {
                        if (realm != null) {
                            realm.close();
                        }
                        //Remove call from static ArrayList of Retrofit Calls
                        removeCall();
                    }

                } else {
                    removeCall();
                    EventBus.getDefault().post(new GenericBus(Config.GET_USER, true, response.errorBody().toString()).object(requesterID));
                }
            }

            @Override
            public void onFailure(Throwable t) {
                //Remove call from static ArrayList of Retrofit Calls
                removeCall();

                //Closing realm instance when CANCEL() method has been called
                if (realm != null) {
                    realm.close();
                }
            }

            //Remove call from static ArrayList of Retrofit Calls
            private void removeCall() {
                calls.remove(userCall);
            }
        });
    }

And here the approach to cancel request and close realm instance in OnFailure method.

public void logoff() {
        for (int i = 0; i < Service.getCalls().size(); i++) {
            Service.getCalls().get(i).cancel();
        }
        RealmConfiguration realmConfig = realm.getConfiguration();
        realm.close();
        user = null;
        Realm.deleteRealm(realmConfig);
    }

I'm not sure if this approach was the best, but works well for me until now. I have to test more and if I realize any problem I will inform.

Tks buddies

renannery commented 9 years ago

Just adding an other point of view,

Would be nice if realm has a method to close all open instances when we are trying to delete realm file once I know that some data can be corrupted but whats the problems if I really want to delete this file?

beeender commented 9 years ago

@renannery We did have discussion about forceClose interface you asked. But we cannot figure out a way to make it harmless.

As you know, every Realm operation relies on a living Realm instance. So, let's say if a query happens on a died Realm instance, an exception will be thrown because of the query interface cannot return any harmless value to satisfy user's logic which might cause more serious not-easy-detectable issues.

By supplying that, user has to catch the RealmIsClose RuntimeException on every other thread to ensure that app won't crash because of a Realm instance is forced to be closed.

So, sorry, we will keep on trying to figure out if there is better way to deal with the situation, but for now, a forceToClose interface doesn't seem to be a right direction to go.

renannery commented 9 years ago

@beeender Tks for reply and you are right, but I hope that we figure this out and I know that we will!

beeender commented 8 years ago

We might want to re-consider this use case. Adding a force param to deleteRealm to do a force deletion without checking the Realm is opened does make sense. The linux FS can ensure that other opened handle still working correctly which means other opened Realm instance will just work until they get closed. The content of file still on the disk and can be accessed by the opened FILE handle.

So, we could:

renannery commented 8 years ago

:+1:

cmelchior commented 8 years ago

@beeender Not totally against the idea, but what would happen in the following case?

1) Open Realm on background thread 2) Delete Realm from main thread 3) Open Realm from main thread (What happens here?) 4) Close Realms on both threads 5) What is the state of the file here?

beeender commented 8 years ago

1) Open Realm on background thread 2) Delete Realm from main thread 3) Open Realm from main thread (What happens here?)

If the Realm is the first instance (all instance has been closed) in main thread, a new Realm will be created. If there is still some a new instance opened in main thread, the reference will be fetched from cache. Main thread still get the file pointed to the old inode. We might be able to clear the cache when deleting Realm to make ShareGroup create a new db file, but still, that means user has a leaked Realm instance potentially. It might be useful for some user case, like clean up on exit or re-login without waiting for background threads returns. User needs to know what would happen and take his own risk.

4) Close Realms on both threads 5) What is the state of the file here?

The deleted one will just gone, the inode of the Realm file will be removed from the kernel. And the newly created one will be kept in the file system.