From 56d8a6d91bc6cee9c0f8fa2caf28d938fbd83ca9 Mon Sep 17 00:00:00 2001 From: ylecollen Date: Wed, 11 Jan 2017 14:33:08 +0100 Subject: [PATCH] improve_e2e_thread There is no more CryptoStore thread. --- .../org/matrix/androidsdk/CryptoTest.java | 61 ------ .../matrix/androidsdk/crypto/MXCrypto.java | 101 ++++----- .../data/cryptostore/MXFileCryptoStore.java | 195 ++++-------------- 3 files changed, 88 insertions(+), 269 deletions(-) diff --git a/matrix-sdk/src/androidTest/java/org/matrix/androidsdk/CryptoTest.java b/matrix-sdk/src/androidTest/java/org/matrix/androidsdk/CryptoTest.java index b2e2a8590..652df9ae6 100644 --- a/matrix-sdk/src/androidTest/java/org/matrix/androidsdk/CryptoTest.java +++ b/matrix-sdk/src/androidTest/java/org/matrix/androidsdk/CryptoTest.java @@ -414,18 +414,6 @@ public void onUnexpectedError(Exception e) { assertTrue(results.containsKey("setDeviceVerification1")); assertTrue (aliceDeviceFromBobPOV.mVerified == MXDeviceInfo.DEVICE_VERIFICATION_BLOCKED); - // the device informations are saved in background thread so give a breath to save everything - MXFileCryptoStore bobFileStore = (MXFileCryptoStore)mBobSession.getCrypto().getCryptoStore(); - - final CountDownLatch lock3b = new CountDownLatch(1); - bobFileStore.getThreadHandler().post(new Runnable() { - @Override - public void run() { - lock3b.countDown(); - } - }); - lock3b.await(1000, TimeUnit.DAYS.MILLISECONDS); - Credentials bobCredentials = mBobSession.getCredentials(); Uri uri = Uri.parse(CryptoTestHelper.TESTS_HOME_SERVER_URL); @@ -3133,31 +3121,6 @@ public void onLiveEvent(Event event, RoomState roomState) { assertTrue(statuses + "", statuses.containsKey("AliceJoin")); mBobSession.getDataHandler().removeListener(bobEventListener); - - if (cryptedBob) { - // the crypto store data is saved in background thread - // so add a delay to let save the data - MXFileCryptoStore bobFileCryptoStore = (MXFileCryptoStore) mBobSession.getCrypto().getCryptoStore(); - MXFileCryptoStore aliceFileCryptoStore = (MXFileCryptoStore) mAliceSession.getCrypto().getCryptoStore(); - - final CountDownLatch lock3 = new CountDownLatch(2); - - bobFileCryptoStore.getThreadHandler().post(new Runnable() { - @Override - public void run() { - lock3.countDown(); - } - }); - - aliceFileCryptoStore.getThreadHandler().post(new Runnable() { - @Override - public void run() { - lock3.countDown(); - } - }); - - lock3.await(1000, TimeUnit.DAYS.MILLISECONDS); - } } private void doE2ETestWithAliceAndBobAndSamInARoom() throws Exception { @@ -3377,30 +3340,6 @@ public void onToDeviceEvent(Event event) { roomFromAlicePOV.sendEvent(buildTextEvent(messagesFromAlice.get(1), mAliceSession), callback); lock.await(1000, TimeUnit.DAYS.MILLISECONDS); assertTrue(mMessagesCount == 5); - - // the crypto store data is saved in background thread - // so add a delay to let save the data - - MXFileCryptoStore bobFileCryptoStore = (MXFileCryptoStore)mBobSession.getCrypto().getCryptoStore(); - MXFileCryptoStore aliceFileCryptoStore = (MXFileCryptoStore)mAliceSession.getCrypto().getCryptoStore(); - - final CountDownLatch lock3 = new CountDownLatch(2); - - bobFileCryptoStore.getThreadHandler().post(new Runnable() { - @Override - public void run() { - lock3.countDown(); - } - }); - - aliceFileCryptoStore.getThreadHandler().post(new Runnable() { - @Override - public void run() { - lock3.countDown(); - } - }); - - lock3.await(1000, TimeUnit.DAYS.MILLISECONDS); } private boolean checkEncryptedEvent(Event event, String roomId, String clearMessage, MXSession senderSession) throws Exception { diff --git a/matrix-sdk/src/main/java/org/matrix/androidsdk/crypto/MXCrypto.java b/matrix-sdk/src/main/java/org/matrix/androidsdk/crypto/MXCrypto.java index 6f22a86f5..aff8f9e77 100755 --- a/matrix-sdk/src/main/java/org/matrix/androidsdk/crypto/MXCrypto.java +++ b/matrix-sdk/src/main/java/org/matrix/androidsdk/crypto/MXCrypto.java @@ -421,57 +421,62 @@ public void run() { @Override public void onSuccess(Void info) { - if (!hasBeenReleased()) { - Log.d(LOG_TAG, "###########################################################"); - Log.d(LOG_TAG, "uploadKeys done for " + mSession.getMyUserId()); - Log.d(LOG_TAG, " - device id : " + mSession.getCredentials().deviceId); - Log.d(LOG_TAG, " - ed25519 : " + mOlmDevice.getDeviceEd25519Key()); - Log.d(LOG_TAG, " - curve25519 : " + mOlmDevice.getDeviceCurve25519Key()); - Log.d(LOG_TAG, " - oneTimeKeys: " + mLastPublishedOneTimeKeys); // They are - Log.d(LOG_TAG, ""); - - checkDeviceAnnounced(new ApiCallback() { - @Override - public void onSuccess(Void info) { - if (null != mNetworkConnectivityReceiver) { - mNetworkConnectivityReceiver.removeEventListener(mNetworkListener); - } - - mIsStarting = false; - mIsStarted = true; - startUploadKeysTimer(true); - - for (ApiCallback callback : mInitializationCallbacks) { - final ApiCallback fCallback = callback; - getUIHandler().post(new Runnable() { - @Override - public void run() { - fCallback.onSuccess(null); + getEncryptingThreadHandler().post(new Runnable() { + @Override + public void run() { + if (!hasBeenReleased()) { + Log.d(LOG_TAG, "###########################################################"); + Log.d(LOG_TAG, "uploadKeys done for " + mSession.getMyUserId()); + Log.d(LOG_TAG, " - device id : " + mSession.getCredentials().deviceId); + Log.d(LOG_TAG, " - ed25519 : " + mOlmDevice.getDeviceEd25519Key()); + Log.d(LOG_TAG, " - curve25519 : " + mOlmDevice.getDeviceCurve25519Key()); + Log.d(LOG_TAG, " - oneTimeKeys: " + mLastPublishedOneTimeKeys); // They are + Log.d(LOG_TAG, ""); + + checkDeviceAnnounced(new ApiCallback() { + @Override + public void onSuccess(Void info) { + if (null != mNetworkConnectivityReceiver) { + mNetworkConnectivityReceiver.removeEventListener(mNetworkListener); } - }); - } - mInitializationCallbacks.clear(); - } - - @Override - public void onNetworkError(Exception e) { - Log.e(LOG_TAG, "## start failed : " + e.getMessage()); - onError(); - } - @Override - public void onMatrixError(MatrixError e) { - Log.e(LOG_TAG, "## start failed : " + e.getMessage()); - onError(); - } - - @Override - public void onUnexpectedError(Exception e) { - Log.e(LOG_TAG, "## start failed : " + e.getMessage()); - onError(); + mIsStarting = false; + mIsStarted = true; + startUploadKeysTimer(true); + + for (ApiCallback callback : mInitializationCallbacks) { + final ApiCallback fCallback = callback; + getUIHandler().post(new Runnable() { + @Override + public void run() { + fCallback.onSuccess(null); + } + }); + } + mInitializationCallbacks.clear(); + } + + @Override + public void onNetworkError(Exception e) { + Log.e(LOG_TAG, "## start failed : " + e.getMessage()); + onError(); + } + + @Override + public void onMatrixError(MatrixError e) { + Log.e(LOG_TAG, "## start failed : " + e.getMessage()); + onError(); + } + + @Override + public void onUnexpectedError(Exception e) { + Log.e(LOG_TAG, "## start failed : " + e.getMessage()); + onError(); + } + }); } - }); - } + } + }); } @Override diff --git a/matrix-sdk/src/main/java/org/matrix/androidsdk/data/cryptostore/MXFileCryptoStore.java b/matrix-sdk/src/main/java/org/matrix/androidsdk/data/cryptostore/MXFileCryptoStore.java index 776dd3025..0848ff1f5 100644 --- a/matrix-sdk/src/main/java/org/matrix/androidsdk/data/cryptostore/MXFileCryptoStore.java +++ b/matrix-sdk/src/main/java/org/matrix/androidsdk/data/cryptostore/MXFileCryptoStore.java @@ -127,14 +127,9 @@ public class MXFileCryptoStore implements IMXCryptoStore { private File mInboundGroupSessionsFileTmp; private File mInboundGroupSessionsFolder; - // tell if the store is corrupted private boolean mIsCorrupted = false; - // the background thread - private HandlerThread mHandlerThread = null; - private MXOsHandler mFileStoreHandler = null; - public MXFileCryptoStore() { } @@ -278,27 +273,6 @@ public String getDeviceId() { return mMetaData.mDeviceId; } - /** - * @return the thread handler - */ - public MXOsHandler getThreadHandler() { - if (null == mFileStoreHandler) { - if (null == mHandlerThread) { - mHandlerThread = new HandlerThread("MXFileCryptoStoreBackgroundThread_" + mCredentials.userId + System.currentTimeMillis(), Thread.MIN_PRIORITY); - mHandlerThread.start(); - } - - // GA issue - if (null != mHandlerThread.getLooper()) { - mFileStoreHandler = new MXOsHandler(mHandlerThread.getLooper()); - } else { - return new MXOsHandler(Looper.getMainLooper()); - } - } - - return mFileStoreHandler; - } - /** * Store a serializable object into a dedicated file. * @param object the object to write. @@ -306,6 +280,11 @@ public MXOsHandler getThreadHandler() { * @param description the object description */ private void storeObject(Object object, File file, String description) { + + if (Thread.currentThread() == Looper.getMainLooper().getThread()) { + Log.e(LOG_TAG, "## storeObject() : should not be called in the UI thread " + description); + } + synchronized (LOG_TAG) { try { long t0 = System.currentTimeMillis(); @@ -386,11 +365,14 @@ public boolean deviceAnnounced() { @Override public void storeUserDevice(String userId, MXDeviceInfo device) { + final HashMap devicesMap; + synchronized (mUsersDevicesInfoMapLock) { mUsersDevicesInfoMap.setObject(device, userId, device.deviceId); + devicesMap = new HashMap<>(mUsersDevicesInfoMap.getMap().get(userId)); } - flushDevicesForUser(userId); + storeObject(devicesMap, new File(mDevicesFolder, userId), "storeUserDevice " + userId); } @Override @@ -410,22 +392,7 @@ public void storeUserDevices(String userId, Map devices) { mUsersDevicesInfoMap.setObjects(devices, userId); } - flushDevicesForUser(userId); - } - - /** - * Flush the devices list for an userId - * @param userId the userId - */ - private void flushDevicesForUser(final String userId) { - final HashMap devicesMap = cloneUserDevicesInfoMap(userId); - - getThreadHandler().post(new Runnable() { - @Override - public void run() { - storeObject(devicesMap, new File(mDevicesFolder, userId), "flushDevicesForUser " + userId); - } - }); + storeObject(devices, new File(mDevicesFolder, userId), "storeUserDevice " + userId); } @Override @@ -448,34 +415,21 @@ public void storeRoomAlgorithm(String roomId, String algorithm) { if ((null != roomId) && (null != algorithm)) { mRoomsAlgorithms.put(roomId, algorithm); - try { - final HashMap roomsAlgorithms = new HashMap<>(mRoomsAlgorithms); - - getThreadHandler().post(new Runnable() { - @Override - public void run() { - // the file is temporary copied to avoid loosing data if the application crashes. - - // delete the previous tmp - if (mAlgorithmsFileTmp.exists()) { - mAlgorithmsFileTmp.delete(); - } + // delete the previous tmp + if (mAlgorithmsFileTmp.exists()) { + mAlgorithmsFileTmp.delete(); + } - // copy the existing file - if (mAlgorithmsFile.exists()) { - mAlgorithmsFile.renameTo(mAlgorithmsFileTmp); - } + // copy the existing file + if (mAlgorithmsFile.exists()) { + mAlgorithmsFile.renameTo(mAlgorithmsFileTmp); + } - storeObject(roomsAlgorithms, mAlgorithmsFile, "storeAlgorithmForRoom - in background"); + storeObject(mRoomsAlgorithms, mAlgorithmsFile, "storeAlgorithmForRoom - in background"); - // remove the tmp file - if (mAlgorithmsFileTmp.exists()) { - mAlgorithmsFileTmp.delete(); - } - } - }); - } catch (OutOfMemoryError oom) { - Log.e(LOG_TAG, "## storeAlgorithmForRoom() : oom"); + // remove the tmp file + if (mAlgorithmsFileTmp.exists()) { + mAlgorithmsFileTmp.delete(); } } } @@ -507,12 +461,6 @@ public void storeSession(final OlmSession olmSession, final String deviceKey) { mOlmSessions.put(deviceKey, new HashMap()); } - final File keyFolder = new File(mOlmSessionsFolder, encodeFilename(deviceKey)); - - if (!keyFolder.exists()) { - keyFolder.mkdir(); - } - OlmSession prevOlmSession = mOlmSessions.get(deviceKey).get(sessionIdentifier); // test if the session is a new one @@ -522,21 +470,15 @@ public void storeSession(final OlmSession olmSession, final String deviceKey) { } mOlmSessions.get(deviceKey).put(sessionIdentifier, olmSession); } + } - try { - final File fOlmFile = new File(keyFolder, encodeFilename(sessionIdentifier)); - final String fSessionIdentifier = sessionIdentifier; + final File keyFolder = new File(mOlmSessionsFolder, encodeFilename(deviceKey)); - getThreadHandler().post(new Runnable() { - @Override - public void run() { - storeObject(olmSession, fOlmFile, "Store olm session " + deviceKey + " " + fSessionIdentifier); - } - }); - } catch (OutOfMemoryError oom) { - Log.e(LOG_TAG, "## flushSessions() : oom"); - } + if (!keyFolder.exists()) { + keyFolder.mkdir(); } + + storeObject(olmSession, new File(keyFolder, encodeFilename(sessionIdentifier)), "Store olm session " + deviceKey + " " + sessionIdentifier); } } @@ -611,24 +553,17 @@ public void storeInboundGroupSession(final MXOlmInboundGroupSession session) { // update the map mInboundGroupSessions.get(session.mSenderKey).put(sessionIdentifier, session); } + } - Log.d(LOG_TAG, "## storeInboundGroupSession() : store session " + sessionIdentifier); - - File senderKeyFolder = new File(mInboundGroupSessionsFolder, encodeFilename(session.mSenderKey)); - - if (!senderKeyFolder.exists()) { - senderKeyFolder.mkdir(); - } + Log.d(LOG_TAG, "## storeInboundGroupSession() : store session " + sessionIdentifier); - final File inboundSessionFile = new File(senderKeyFolder, encodeFilename(sessionIdentifier)); + File senderKeyFolder = new File(mInboundGroupSessionsFolder, encodeFilename(session.mSenderKey)); - getThreadHandler().post(new Runnable() { - @Override - public void run() { - storeObject(session, inboundSessionFile, "storeInboundGroupSession - in background"); - } - }); + if (!senderKeyFolder.exists()) { + senderKeyFolder.mkdir(); } + + storeObject(session, new File(senderKeyFolder, encodeFilename(sessionIdentifier)), "storeInboundGroupSession - in background"); } } @@ -687,11 +622,6 @@ private void resetData() { deleteStore(); } - if (null != mHandlerThread) { - mHandlerThread.quit(); - mHandlerThread = null; - } - if (!mStoreFile.exists()) { mStoreFile.mkdirs(); } @@ -1008,61 +938,6 @@ private void preloadCryptoData() { } } - /** - * @return clone the device infos map - */ - private HashMap cloneUserDevicesInfoMap(String user) { - HashMap clone = new HashMap<>(); - - synchronized (mUsersDevicesInfoMapLock) { - HashMap source = mUsersDevicesInfoMap.getMap().get(user); - - if (null != source) { - Set deviceIds = source.keySet(); - for (String deviceId : deviceIds) { - clone.put(deviceId, cloneDeviceInfo(source.get(deviceId))); - } - } - } - - return clone; - } - - /** - * @return a deep copy - */ - public static MXDeviceInfo cloneDeviceInfo(MXDeviceInfo di) { - MXDeviceInfo copy = new MXDeviceInfo(di.deviceId); - - copy.userId = di.userId; - - if (null != di.algorithms) { - copy.algorithms = new ArrayList<>(di.algorithms); - } - - if (null != di.keys) { - copy.keys = new HashMap<>(di.keys); - } - - if (null != di.signatures) { - copy.signatures = new HashMap<>(); - - Set keySet = di.signatures.keySet(); - - for(String k : keySet) { - copy.signatures.put(k, new HashMap<>(di.signatures.get(k))); - } - } - - if (null != di.unsigned) { - copy.unsigned = new HashMap<>(di.unsigned); - } - - copy.mVerified = di.mVerified; - - return copy; - } - final private static char[] hexArray = "0123456789ABCDEF".toCharArray(); /**