Skip to content

Commit

Permalink
Process safe deleteRealm and assetFile (#5417)
Browse files Browse the repository at this point in the history
- Use SharedRealm::call_with_lock() to make sure deleteRealm and
  assetFile calls are process safe.
- Also those calls will throw if the Realm instance on the sync client
  thread is still opened.
  • Loading branch information
beeender authored Oct 17, 2017
1 parent c1b4193 commit f80f06d
Show file tree
Hide file tree
Showing 21 changed files with 338 additions and 105 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
## 4.1.0 (YYYY-MM-DD)

## Breaking Changes

## Enhancements

* `Realm.deleteRealm()` and `RealmConfiguration.assetFile()` are multi-processes safe now.

## Bug Fixes

* Fix some potential database corruption caused by deleting the Realm file while a Realm instance are still opened in another process or the sync client thread.

## Internal

### Credits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2301,8 +2301,11 @@ public void run() {

assertTrue(Realm.deleteRealm(configuration));

// Directory should be empty now.
assertEquals(0, tempDir.listFiles().length);
assertEquals(1, tempDir.listFiles().length);

// Lock file should never be deleted
File lockFile = new File(configuration.getPath() + ".lock");
assertTrue(lockFile.exists());
}

// Tests that all methods that require a transaction. (ie. any function that mutates Realm data)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,13 @@ public class CollectionTests {

private final long[] oneNullTable = new long[] {NativeObject.NULLPTR};

private RealmConfiguration config;
private SharedRealm sharedRealm;
private Table table;

@Before
public void setUp() {
config = configFactory.createConfiguration();
sharedRealm = getSharedRealm();
populateData();
populateData(sharedRealm);
}

@After
Expand All @@ -74,6 +72,16 @@ public void tearDown() {
}

private SharedRealm getSharedRealm() {
RealmConfiguration config = configFactory.createConfiguration();
return getSharedRealm(config);
}

private SharedRealm getSharedRealmForLooper() {
RealmConfiguration config = looperThread.createConfiguration();
return getSharedRealm(config);
}

private SharedRealm getSharedRealm(RealmConfiguration config) {
OsRealmConfig.Builder configBuilder = new OsRealmConfig.Builder(config)
.autoUpdateNotification(true);
SharedRealm sharedRealm = SharedRealm.getInstance(configBuilder);
Expand All @@ -87,7 +95,7 @@ private Table getTable(SharedRealm sharedRealm) {
return sharedRealm.getTable(Table.getTableNameForClass("test_table"));
}

private void populateData() {
private void populateData(SharedRealm sharedRealm) {
sharedRealm.beginTransaction();
table = sharedRealm.createTable(Table.getTableNameForClass("test_table"));
// Specify the column types and names
Expand Down Expand Up @@ -119,12 +127,13 @@ private void populateData() {
sharedRealm.commitTransaction();
}

private void addRowAsync() {
private void addRowAsync(final SharedRealm sharedRealm) {
final CountDownLatch latch = new CountDownLatch(1);
final RealmConfiguration configuration = sharedRealm.getConfiguration();
new Thread(new Runnable() {
@Override
public void run() {
SharedRealm sharedRealm = getSharedRealm();
SharedRealm sharedRealm = getSharedRealm(configuration);
addRow(sharedRealm);
sharedRealm.close();
latch.countDown();
Expand Down Expand Up @@ -253,7 +262,8 @@ public void distinct() {
@Test
@RunTestInLooperThread
public void addListener_shouldBeCalledToReturnTheQueryResults() {
final SharedRealm sharedRealm = getSharedRealm();
final SharedRealm sharedRealm = getSharedRealmForLooper();
populateData(sharedRealm);
Table table = getTable(sharedRealm);

final Collection collection = new Collection(sharedRealm, table.where());
Expand Down Expand Up @@ -332,7 +342,7 @@ public void onChange(Collection element) {
}
});

addRowAsync();
addRowAsync(sharedRealm);

sharedRealm.waitForChange();
sharedRealm.refresh();
Expand All @@ -342,7 +352,8 @@ public void onChange(Collection element) {
@Test
@RunTestInLooperThread
public void addListener_queryNotReturned() {
final SharedRealm sharedRealm = getSharedRealm();
final SharedRealm sharedRealm = getSharedRealmForLooper();
populateData(sharedRealm);
Table table = getTable(sharedRealm);

final Collection collection = new Collection(sharedRealm, table.where());
Expand All @@ -357,13 +368,14 @@ public void onChange(Collection collection1) {
}
});

addRowAsync();
addRowAsync(sharedRealm);
}

@Test
@RunTestInLooperThread
public void addListener_queryReturned() {
final SharedRealm sharedRealm = getSharedRealm();
final SharedRealm sharedRealm = getSharedRealmForLooper();
populateData(sharedRealm);
Table table = getTable(sharedRealm);

final Collection collection = new Collection(sharedRealm, table.where());
Expand All @@ -379,15 +391,16 @@ public void onChange(Collection collection1) {
}
});

addRowAsync();
addRowAsync(sharedRealm);
}

// Local commit will trigger the listener first when beginTransaction gets called then again when transaction
// committed.
@Test
@RunTestInLooperThread
public void addListener_triggeredByLocalCommit() {
final SharedRealm sharedRealm = getSharedRealm();
final SharedRealm sharedRealm = getSharedRealmForLooper();
populateData(sharedRealm);
Table table = getTable(sharedRealm);
final AtomicInteger listenerCounter = new AtomicInteger(0);

Expand Down Expand Up @@ -468,7 +481,8 @@ public void collectionIterator_invalid_nonLooperThread_byRefresh() {
@Test
@RunTestInLooperThread
public void collectionIterator_invalid_looperThread_byRemoteTransaction() {
final SharedRealm sharedRealm = getSharedRealm();
final SharedRealm sharedRealm = getSharedRealmForLooper();
populateData(sharedRealm);
Table table = getTable(sharedRealm);
final Collection collection = new Collection(sharedRealm, table.where());
final TestIterator iterator = new TestIterator(collection);
Expand All @@ -487,7 +501,7 @@ public void onChange(Collection element) {
}
});

addRowAsync();
addRowAsync(sharedRealm);
}

@Test
Expand Down Expand Up @@ -522,6 +536,9 @@ public void onChange(Collection element) {
@Test
@RunTestInLooperThread
public void load() {
final SharedRealm sharedRealm = getSharedRealmForLooper();
looperThread.closeAfterTest(sharedRealm);
populateData(sharedRealm);
final Collection collection = new Collection(sharedRealm, table.where());
collection.addListener(collection, new RealmChangeListener<Collection>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import android.support.test.InstrumentationRegistry;
import android.support.test.runner.AndroidJUnit4;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -39,16 +40,13 @@ public class JNIColumnInfoTest {
@Rule
public final TestRealmConfigurationFactory configFactory = new TestRealmConfigurationFactory();

@SuppressWarnings("FieldCanBeLocal")
private RealmConfiguration config;
@SuppressWarnings("FieldCanBeLocal")
private SharedRealm sharedRealm;
private Table table;

@Before
public void setUp() {
Realm.init(InstrumentationRegistry.getInstrumentation().getContext());
config = configFactory.createConfiguration();
RealmConfiguration config = configFactory.createConfiguration();
sharedRealm = SharedRealm.getInstance(config);

table = TestHelper.createTable(sharedRealm, "temp", new TestHelper.AdditionalTableSetup() {
Expand All @@ -60,6 +58,13 @@ public void execute(Table table) {
});
}

@After
public void tearDown() {
if (sharedRealm != null) {
sharedRealm.close();
}
}

@Test
public void shouldGetColumnInformation() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import android.support.test.runner.AndroidJUnit4;
import android.util.Pair;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -60,6 +61,13 @@ public void setUp() {
sharedRealm = SharedRealm.getInstance(config);
}

@After
public void tearDown() {
if (sharedRealm != null) {
sharedRealm.close();
}
}

@Test
public void tableToString() {
Table t = TestHelper.createTable(sharedRealm, "temp", new TestHelper.AdditionalTableSetup() {
Expand Down Expand Up @@ -307,10 +315,6 @@ public void execute(Table t) {
@Test
public void getName() {
String TABLE_NAME = "tableName";
RealmConfiguration configuration = configFactory.createConfiguration();
Realm.deleteRealm(configuration);

SharedRealm sharedRealm = SharedRealm.getInstance(configuration);
//noinspection TryFinallyCanBeTryWithResources
try {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright 2017 Realm Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.realm.internal;

import android.support.test.runner.AndroidJUnit4;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;

import java.util.concurrent.atomic.AtomicBoolean;

import io.realm.RealmConfiguration;
import io.realm.rule.TestRealmConfigurationFactory;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
import static junit.framework.Assert.fail;

// Tests for OsObjectStore
@RunWith(AndroidJUnit4.class)
public class OsObjectStoreTests {
@Rule
public final TestRealmConfigurationFactory configFactory = new TestRealmConfigurationFactory();
@Rule
public final ExpectedException thrown = ExpectedException.none();

@Test
public void callWithLock() {
RealmConfiguration config = configFactory.createConfiguration();

// Return false if there are opened SharedRealm instance
SharedRealm sharedRealm = SharedRealm.getInstance(config);
assertFalse(OsObjectStore.callWithLock(config, new Runnable() {
@Override
public void run() {
fail();
}
}));
sharedRealm.close();

final AtomicBoolean callbackCalled = new AtomicBoolean(false);
assertTrue(OsObjectStore.callWithLock(config, new Runnable() {
@Override
public void run() {
callbackCalled.set(true);
}
}));
assertTrue(callbackCalled.get());
}

// Test if a java exception can be thrown from the callback.
@Test
public void callWithLock_throwInCallback() {
RealmConfiguration config = configFactory.createConfiguration();
final RuntimeException exception = new RuntimeException();

try {
OsObjectStore.callWithLock(config, new Runnable() {
@Override
public void run() {
throw exception;
}
});
fail();
} catch (RuntimeException e) {
assertEquals(exception, e);
}

// The lock should be released after exception thrown
final AtomicBoolean callbackCalled = new AtomicBoolean(false);
assertTrue(OsObjectStore.callWithLock(config, new Runnable() {
@Override
public void run() {
callbackCalled.set(true);
}
}));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,11 @@ public void onChange(SharedRealm sharedRealm) {
}

private void makeRemoteChanges(final RealmConfiguration config) {
new Thread(new Runnable() {
@Override
public void run() {
SharedRealm sharedRealm = getSharedRealm(config);
sharedRealm.beginTransaction();
sharedRealm.commitTransaction();
sharedRealm.close();
}
}).start();
// We don't use cache from RealmCoordinator
SharedRealm sharedRealm = getSharedRealm(config);
sharedRealm.beginTransaction();
sharedRealm.commitTransaction();
sharedRealm.close();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ protected void after() {

@Override
public Statement apply(Statement base, Description description) {
setTestName(description);
final RunTestInLooperThread annotation = description.getAnnotation(RunTestInLooperThread.class);
if (annotation == null) {
return base;
Expand Down
Loading

0 comments on commit f80f06d

Please sign in to comment.