Skip to content

Commit 238946d

Browse files
Copilotanod
andauthored
Fix thread-safety in initPageSize() using double-checked locking with static lock (#1396)
* Initial plan * Add thread-safety to initPageSize() using double-checked locking Co-authored-by: anod <[email protected]> * Use static lock object for thread-safety in initPageSize() Co-authored-by: anod <[email protected]> * Add concurrent test for thread-safe page size initialization Co-authored-by: anod <[email protected]> * Use ExecutorService instead of raw threads in concurrent test Co-authored-by: anod <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: anod <[email protected]>
1 parent e0f5b08 commit 238946d

File tree

2 files changed

+117
-14
lines changed

2 files changed

+117
-14
lines changed

lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/OfflineRoomUnitTest.java

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,20 @@
1717
import org.junit.Test;
1818
import org.junit.runner.RunWith;
1919

20+
import java.util.ArrayList;
21+
import java.util.List;
22+
import java.util.concurrent.CountDownLatch;
23+
import java.util.concurrent.ExecutorService;
24+
import java.util.concurrent.Executors;
25+
import java.util.concurrent.TimeUnit;
26+
import java.util.concurrent.atomic.AtomicInteger;
27+
2028
import static org.hamcrest.Matchers.*;
2129
import static org.junit.Assert.assertEquals;
2230
import static org.junit.Assert.assertNotEquals;
2331
import static org.junit.Assert.assertNotNull;
2432
import static org.junit.Assert.assertThat;
33+
import static org.junit.Assert.assertTrue;
2534

2635
@RunWith(AndroidJUnit4.class)
2736
public class OfflineRoomUnitTest {
@@ -185,5 +194,94 @@ public void RetireRetries() {
185194
}
186195
}
187196
}
197+
198+
@Test
199+
public void ConcurrentPageSizeInitialization() throws InterruptedException {
200+
Context appContext = InstrumentationRegistry.getInstrumentation().getTargetContext();
201+
try (OfflineRoom room = new OfflineRoom(appContext, "OfflineRoomConcurrent")) {
202+
room.deleteAllRecords();
203+
204+
// Add a record to ensure the database is not empty
205+
StorageRecord record = new StorageRecord(
206+
0, "Test",
207+
StorageRecord.EventLatency_Normal,
208+
StorageRecord.EventPersistence_Normal,
209+
32,
210+
1,
211+
0,
212+
new byte[]{1, 2, 3});
213+
room.storeRecords(record);
214+
215+
final int threadCount = 10;
216+
final CountDownLatch startLatch = new CountDownLatch(1);
217+
final CountDownLatch doneLatch = new CountDownLatch(threadCount);
218+
final List<Long> pageSizes = new ArrayList<>();
219+
final List<Long> totalSizes = new ArrayList<>();
220+
final AtomicInteger errorCount = new AtomicInteger(0);
221+
222+
// Use ExecutorService for proper thread management
223+
ExecutorService executor = Executors.newFixedThreadPool(threadCount);
224+
try {
225+
// Submit tasks that will call loadPageSize() and totalSize() concurrently
226+
for (int i = 0; i < threadCount; i++) {
227+
executor.submit(() -> {
228+
try {
229+
// Wait for all threads to be ready
230+
startLatch.await();
231+
232+
// Call both methods to test concurrent initialization
233+
long pageSize = room.loadPageSize();
234+
long totalSize = room.totalSize();
235+
236+
synchronized (pageSizes) {
237+
pageSizes.add(pageSize);
238+
totalSizes.add(totalSize);
239+
}
240+
} catch (Exception e) {
241+
errorCount.incrementAndGet();
242+
e.printStackTrace();
243+
} finally {
244+
doneLatch.countDown();
245+
}
246+
});
247+
}
248+
249+
// Start all threads simultaneously
250+
startLatch.countDown();
251+
252+
// Wait for all threads to complete
253+
doneLatch.await();
254+
} finally {
255+
// Ensure executor is properly shut down
256+
executor.shutdown();
257+
if (!executor.awaitTermination(5, TimeUnit.SECONDS)) {
258+
executor.shutdownNow();
259+
}
260+
}
261+
262+
// Verify no errors occurred
263+
assertEquals("No errors should occur during concurrent access", 0, errorCount.get());
264+
265+
// Verify all threads got results
266+
assertEquals("All threads should complete", threadCount, pageSizes.size());
267+
assertEquals("All threads should complete", threadCount, totalSizes.size());
268+
269+
// Verify all threads got the same page size (no race condition)
270+
long firstPageSize = pageSizes.get(0);
271+
assertThat("Page size should be valid", firstPageSize, greaterThan(0L));
272+
assertThat("Page size should be a power of 2", firstPageSize & (firstPageSize - 1), is(0L));
273+
274+
for (long pageSize : pageSizes) {
275+
assertEquals("All threads should get the same page size", firstPageSize, pageSize);
276+
}
277+
278+
// Verify all threads got a valid total size
279+
for (long totalSize : totalSizes) {
280+
assertThat("Total size should be valid", totalSize, greaterThan(0L));
281+
}
282+
283+
room.deleteAllRecords();
284+
}
285+
}
188286
}
189287

lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/OfflineRoom.java

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ public Long call()
8989
private OfflineRoomDatabase m_db;
9090
private StorageRecordDao m_srDao;
9191
private StorageSettingDao m_settingDao;
92-
private long m_pageSize = -1;
92+
private volatile long m_pageSize = -1;
9393
private static final long PAGE_SIZE_DEFAULT = 4096;
94+
private static final Object PAGE_SIZE_LOCK = new Object();
9495

9596
public OfflineRoom(Context context, String name) {
9697
RoomDatabase.Builder<OfflineRoomDatabase> builder;
@@ -290,22 +291,26 @@ public void deleteSetting(String name) {
290291

291292
private void initPageSize() {
292293
if (m_pageSize == -1) {
293-
try {
294-
try (Cursor c = m_db.query("PRAGMA page_size", null)) {
295-
if (c.getCount() == 1 && c.getColumnCount() == 1) {
296-
c.moveToFirst();
297-
m_pageSize = c.getLong(0);
298-
} else {
294+
synchronized (PAGE_SIZE_LOCK) {
295+
if (m_pageSize == -1) {
296+
try {
297+
try (Cursor c = m_db.query("PRAGMA page_size", null)) {
298+
if (c.getCount() == 1 && c.getColumnCount() == 1) {
299+
c.moveToFirst();
300+
m_pageSize = c.getLong(0);
301+
} else {
302+
m_pageSize = PAGE_SIZE_DEFAULT;
303+
Log.e("MAE",
304+
String.format("Unexpected result from PRAGMA page_size: %d rows, %d columns",
305+
c.getCount(),
306+
c.getColumnCount()));
307+
}
308+
}
309+
} catch (Exception e) {
299310
m_pageSize = PAGE_SIZE_DEFAULT;
300-
Log.e("MAE",
301-
String.format("Unexpected result from PRAGMA page_size: %d rows, %d columns",
302-
c.getCount(),
303-
c.getColumnCount()));
311+
Log.e("MAE", "Failed to query PRAGMA page_size, using default page size.", e);
304312
}
305313
}
306-
} catch (Exception e) {
307-
m_pageSize = PAGE_SIZE_DEFAULT;
308-
Log.e("MAE", "Failed to query PRAGMA page_size, using default page size.", e);
309314
}
310315
}
311316
}

0 commit comments

Comments
 (0)