From 3c61c8ea2ca267b2b2f547ce7fe9c908eb0b2b8a Mon Sep 17 00:00:00 2001 From: guoling Date: Mon, 30 Dec 2019 21:21:42 +0800 Subject: [PATCH] fix flock() bug on ashmem in Android --- .../mmkvdemo/BenchMarkBaseService.java | 8 +- .../com/tencent/mmkvdemo/MainActivity.java | 6 +- Core/CMakeLists.txt | 1 + Core/InterProcessLock.cpp | 14 ++- Core/InterProcessLock.h | 19 +++- Core/InterProcessLock_Android.cpp | 89 +++++++++++++++++++ Core/InterProcessLock_Win32.cpp | 2 +- Core/MMKV.cpp | 14 +-- Core/MMKV.h | 6 +- Core/MMKVLog.cpp | 11 ++- Core/MMKVLog_Android.cpp | 10 ++- Core/MMKV_Android.cpp | 4 +- Core/PBEncodeItem.hpp | 3 +- Core/aes/AESCrypt.cpp | 4 +- Core/aes/AESCrypt.h | 2 +- 15 files changed, 160 insertions(+), 33 deletions(-) create mode 100644 Core/InterProcessLock_Android.cpp diff --git a/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/BenchMarkBaseService.java b/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/BenchMarkBaseService.java index d7fa9a81..1d50a05b 100644 --- a/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/BenchMarkBaseService.java +++ b/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/BenchMarkBaseService.java @@ -27,6 +27,7 @@ import android.os.IBinder; import android.util.Log; import androidx.annotation.Nullable; import com.tencent.mmkv.MMKV; +import com.tencent.mmkv.MMKVLogLevel; import com.tencent.mmkv.ParcelableMMKV; import java.util.Random; @@ -50,8 +51,8 @@ public abstract class BenchMarkBaseService extends Service { private static final int m_loops = 1000; public static final String MMKV_ID = "benchmark_interprocess"; private static final String SP_ID = "benchmark_interprocess_sp"; - private static final String CryptKey = null; - //private static final String CryptKey = "Tencent MMKV"; + public static final String CryptKey = null; + //public static final String CryptKey = "Tencent MMKV"; @Override public void onCreate() { @@ -59,10 +60,11 @@ public abstract class BenchMarkBaseService extends Service { Log.i("MMKV", "onCreate BenchMarkBaseService"); MMKV.initialize(this); + //MMKV.setLogLevel(MMKVLogLevel.LevelInfo); { long startTime = System.currentTimeMillis(); - MMKV mmkv = MMKV.mmkvWithID(MMKV_ID, MMKV.MULTI_PROCESS_MODE); + MMKV mmkv = MMKV.mmkvWithID(MMKV_ID, MMKV.MULTI_PROCESS_MODE, CryptKey); long endTime = System.currentTimeMillis(); Log.i("MMKV", "load [" + MMKV_ID + "]: " + (endTime - startTime) + " ms"); diff --git a/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/MainActivity.java b/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/MainActivity.java index 732ddaeb..c93068ac 100644 --- a/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/MainActivity.java +++ b/Android/MMKV/mmkvdemo/src/main/java/com/tencent/mmkvdemo/MainActivity.java @@ -84,7 +84,7 @@ public class MainActivity extends AppCompatActivity implements MMKVHandler, MMKV baseline.sharedPreferencesBaselineTest(); baseline.sqliteBaselineTest(); - //testInterProcessReKey(); + testInterProcessReKey(); //testInterProcessLockPhase2(); } }); @@ -92,7 +92,7 @@ public class MainActivity extends AppCompatActivity implements MMKVHandler, MMKV //testHolderForMultiThread(); //prepareInterProcessAshmem(); - //prepareInterProcessAshmemByContentProvider(KEY_1); + prepareInterProcessAshmemByContentProvider(KEY_1); final Button button_read_int = findViewById(R.id.button_read_int); button_read_int.setOnClickListener(new View.OnClickListener() { @@ -148,7 +148,7 @@ public class MainActivity extends AppCompatActivity implements MMKVHandler, MMKV } private void testInterProcessLogic() { - MMKV mmkv = MMKV.mmkvWithID(MyService.MMKV_ID, MMKV.MULTI_PROCESS_MODE); + MMKV mmkv = MMKV.mmkvWithID(MyService.MMKV_ID, MMKV.MULTI_PROCESS_MODE, MyService.CryptKey); mmkv.putInt(MyService.CMD_ID, 1024); Log.d("mmkv in main", "" + mmkv.decodeInt(MyService.CMD_ID)); diff --git a/Core/CMakeLists.txt b/Core/CMakeLists.txt index 72264201..06f82b9e 100644 --- a/Core/CMakeLists.txt +++ b/Core/CMakeLists.txt @@ -69,6 +69,7 @@ add_library(core InterProcessLock.h InterProcessLock.cpp InterProcessLock_Win32.cpp + InterProcessLock_Android.cpp MemoryFile.h MemoryFile.cpp MemoryFile_Android.cpp diff --git a/Core/InterProcessLock.cpp b/Core/InterProcessLock.cpp index 0f59d97a..f48982f4 100644 --- a/Core/InterProcessLock.cpp +++ b/Core/InterProcessLock.cpp @@ -73,6 +73,11 @@ static uint32_t LockType2FlockType(LockType lockType) { } bool FileLock::platformLock(LockType lockType, bool wait, bool unLockFirstIfNeeded) { +# ifdef MMKV_ANDROID + if (m_isAshmem) { + return ashmemLock(lockType, wait, unLockFirstIfNeeded); + } +# endif auto realLockType = LockType2FlockType(lockType); int cmd = wait ? realLockType : (realLockType | LOCK_NB); if (unLockFirstIfNeeded) { @@ -97,7 +102,12 @@ bool FileLock::platformLock(LockType lockType, bool wait, bool unLockFirstIfNeed } } -bool FileLock::platformUnLock(LockType lockType, bool unlockToSharedLock) { +bool FileLock::platformUnLock(bool unlockToSharedLock) { +# ifdef MMKV_ANDROID + if (m_isAshmem) { + return ashmemUnLock(unlockToSharedLock); + } +# endif int cmd = unlockToSharedLock ? LOCK_SH : LOCK_UN; auto ret = flock(m_fd, cmd); if (ret != 0) { @@ -137,7 +147,7 @@ bool FileLock::unlock(LockType lockType) { unlockToSharedLock = true; } } - return platformUnLock(lockType, unlockToSharedLock); + return platformUnLock(unlockToSharedLock); } } // namespace mmkv diff --git a/Core/InterProcessLock.h b/Core/InterProcessLock.h index f81c211e..564d545a 100644 --- a/Core/InterProcessLock.h +++ b/Core/InterProcessLock.h @@ -42,21 +42,34 @@ class FileLock { bool doLock(LockType lockType, bool wait); bool platformLock(LockType lockType, bool wait, bool unLockFirstIfNeeded); - bool platformUnLock(LockType lockType, bool unLockFirstIfNeeded); + bool platformUnLock(bool unLockFirstIfNeeded); #ifndef MMKV_WIN32 bool isFileLockValid() { return m_fd >= 0; } +# ifdef MMKV_ANDROID + const bool m_isAshmem; + struct flock m_lockInfo; + bool ashmemLock(LockType lockType, bool wait, bool unLockFirstIfNeeded); + bool ashmemUnLock(bool unLockFirstIfNeeded); +# endif + #else OVERLAPPED m_overLapped; bool isFileLockValid() { return m_fd != INVALID_HANDLE_VALUE; } -#endif +#endif // MMKV_WIN32 + public: #ifndef MMKV_WIN32 +# ifndef MMKV_ANDROID explicit FileLock(MMKV_FILE_HANDLE fd) : m_fd(fd), m_sharedLockCount(0), m_exclusiveLockCount(0) {} +# else + explicit FileLock(MMKV_FILE_HANDLE fd, bool isAshmem = false); +# endif // MMKV_ANDROID #else explicit FileLock(MMKV_FILE_HANDLE fd) : m_fd(fd), m_overLapped{0}, m_sharedLockCount(0), m_exclusiveLockCount(0) {} -#endif +#endif // MMKV_WIN32 + bool lock(LockType lockType); bool try_lock(LockType lockType); diff --git a/Core/InterProcessLock_Android.cpp b/Core/InterProcessLock_Android.cpp new file mode 100644 index 00000000..571f6e25 --- /dev/null +++ b/Core/InterProcessLock_Android.cpp @@ -0,0 +1,89 @@ +/* + * Tencent is pleased to support the open source community by making + * MMKV available. + * + * Copyright (C) 2019 THL A29 Limited, a Tencent company. + * All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); you may not use + * this file except in compliance with the License. You may obtain a copy of + * the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * 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. + */ + +#include "InterProcessLock.h" + +#ifdef MMKV_ANDROID +# include "MMKVLog.h" +# include +# include + +namespace mmkv { + +FileLock::FileLock(MMKV_FILE_HANDLE fd, bool isAshmem) + : m_fd(fd), m_sharedLockCount(0), m_exclusiveLockCount(0), m_isAshmem(isAshmem) { + m_lockInfo.l_type = F_WRLCK; + m_lockInfo.l_start = 0; + m_lockInfo.l_whence = SEEK_SET; + m_lockInfo.l_len = 0; + m_lockInfo.l_pid = 0; +} + +static short LockType2FlockType(LockType lockType) { + switch (lockType) { + case SharedLockType: + return F_RDLCK; + case ExclusiveLockType: + return F_WRLCK; + } +} + +bool FileLock::ashmemLock(LockType lockType, bool wait, bool unLockFirstIfNeeded) { + m_lockInfo.l_type = LockType2FlockType(lockType); + if (unLockFirstIfNeeded) { + // try lock + auto ret = fcntl(m_fd, F_SETLK, &m_lockInfo); + if (ret == 0) { + return true; + } + // lets be gentleman: unlock my shared-lock to prevent deadlock + auto type = m_lockInfo.l_type; + m_lockInfo.l_type = F_UNLCK; + ret = fcntl(m_fd, F_SETLK, &m_lockInfo); + if (ret != 0) { + MMKVError("fail to try unlock first fd=%d, ret=%d, error:%s", m_fd, ret, strerror(errno)); + } + m_lockInfo.l_type = type; + } + + int cmd = wait ? F_SETLKW : F_SETLK; + auto ret = fcntl(m_fd, cmd, &m_lockInfo); + if (ret != 0) { + MMKVError("fail to lock fd=%d, ret=%d, error:%s", m_fd, ret, strerror(errno)); + return false; + } else { + return true; + } +} + +bool FileLock::ashmemUnLock(bool unlockToSharedLock) { + m_lockInfo.l_type = static_cast(unlockToSharedLock ? F_RDLCK : F_UNLCK); + auto ret = fcntl(m_fd, F_SETLK, &m_lockInfo); + if (ret != 0) { + MMKVError("fail to unlock fd=%d, ret=%d, error:%s", m_fd, ret, strerror(errno)); + return false; + } else { + return true; + } +} + +} // namespace mmkv + +#endif // MMKV_ANDROID diff --git a/Core/InterProcessLock_Win32.cpp b/Core/InterProcessLock_Win32.cpp index f8daa856..0ca2e285 100644 --- a/Core/InterProcessLock_Win32.cpp +++ b/Core/InterProcessLock_Win32.cpp @@ -63,7 +63,7 @@ bool FileLock::platformLock(LockType lockType, bool wait, bool unLockFirstIfNeed } } -bool FileLock::platformUnLock(LockType lockType, bool unlockToSharedLock) { +bool FileLock::platformUnLock(bool unlockToSharedLock) { /* quote from MSDN: * If the same range is locked with an exclusive and a shared lock, * two unlock operations are necessary to unlock the region; diff --git a/Core/MMKV.cpp b/Core/MMKV.cpp index 986909a7..4defe828 100644 --- a/Core/MMKV.cpp +++ b/Core/MMKV.cpp @@ -239,7 +239,7 @@ void MMKV::loadFromFile() { } if (m_crypter) { if (m_metaInfo.m_version >= MMKVVersionRandomIV) { - m_crypter->reset(m_metaInfo.m_vector, sizeof(m_metaInfo.m_vector)); + m_crypter->resetIV(m_metaInfo.m_vector, sizeof(m_metaInfo.m_vector)); } } @@ -431,7 +431,8 @@ void MMKV::checkLoadData() { loadFromFile(); notifyContentChanged(); } else if (m_metaInfo.m_crcDigest != metaInfo.m_crcDigest) { - MMKVDebug("[%s] oldCrc %u, newCrc %u", m_mmapID.c_str(), m_metaInfo.m_crcDigest, metaInfo.m_crcDigest); + MMKVDebug("[%s] oldCrc %u, newCrc %u, new actualSize %u", m_mmapID.c_str(), m_metaInfo.m_crcDigest, + metaInfo.m_crcDigest, metaInfo.m_actualSize); SCOPEDLOCK(m_sharedProcessLock); size_t fileSize = m_file.getActualFileSize(); @@ -486,7 +487,7 @@ void MMKV::clearAll() { unsigned char newIV[AES_KEY_LEN]; AESCrypt::fillRandomIV(newIV); if (m_crypter) { - m_crypter->reset(newIV, sizeof(newIV)); + m_crypter->resetIV(newIV, sizeof(newIV)); } writeActualSize(0, 0, newIV, IncreaseSequence); m_metaFile.msync(MMKV_SYNC); @@ -508,9 +509,9 @@ void MMKV::clearMemoryCache() { if (m_crypter) { if (m_metaInfo.m_version >= MMKVVersionRandomIV) { - m_crypter->reset(m_metaInfo.m_vector, sizeof(m_metaInfo.m_vector)); + m_crypter->resetIV(m_metaInfo.m_vector, sizeof(m_metaInfo.m_vector)); } else { - m_crypter->reset(); + m_crypter->resetIV(); } } @@ -822,7 +823,7 @@ bool MMKV::doFullWriteBack(MMBuffer &&allData) { if (m_crypter) { #endif AESCrypt::fillRandomIV(newIV); - m_crypter->reset(newIV, sizeof(newIV)); + m_crypter->resetIV(newIV, sizeof(newIV)); auto ptr = allData.getPtr(); m_crypter->encrypt(ptr, ptr, allData.length()); } @@ -899,7 +900,6 @@ bool MMKV::reKey(const string &cryptKey) { return false; } -// TODO: this looks like wrong after iv changes void MMKV::checkReSetCryptKey(const string *cryptKey) { SCOPEDLOCK(m_lock); diff --git a/Core/MMKV.h b/Core/MMKV.h index 2cdd02e7..ae4f88ec 100644 --- a/Core/MMKV.h +++ b/Core/MMKV.h @@ -169,7 +169,7 @@ public: int ashmemFD() { return (m_file.m_fileType & mmkv::MMFILE_TYPE_ASHMEM) ? m_file.getFd() : -1; } int ashmemMetaFD() { return (m_file.m_fileType & mmkv::MMFILE_TYPE_ASHMEM) ? m_metaFile.getFd() : -1; } -#endif +#endif // MMKV_ANDROID static void onExit(); @@ -224,7 +224,7 @@ public: mmkv::MMBuffer getBytes(MMKV_KEY_TYPE key); bool getVector(MMKV_KEY_TYPE key, std::vector &result); -#endif +#endif // MMKV_IOS_OR_MAC bool getBool(MMKV_KEY_TYPE key, bool defaultValue = false); @@ -269,7 +269,7 @@ public: std::vector allKeys(); void removeValuesForKeys(const std::vector &arrKeys); -#endif +#endif // MMKV_IOS_OR_MAC void removeValueForKey(MMKV_KEY_TYPE key); diff --git a/Core/MMKVLog.cpp b/Core/MMKVLog.cpp index b113c3e0..0efb2c2c 100644 --- a/Core/MMKVLog.cpp +++ b/Core/MMKVLog.cpp @@ -38,7 +38,7 @@ MMKV_NAMESPACE_END using namespace mmkv; -static const char *_getFileName(const char *path) { +const char *_getFileName(const char *path) { const char *ptr = strrchr(path, '/'); if (!ptr) { ptr = strrchr(path, '\\'); @@ -51,6 +51,7 @@ static const char *_getFileName(const char *path) { } # ifndef MMKV_ANDROID + static const char *MMKVLogLevelDesc(MMKVLogLevel level) { switch (level) { case MMKVLogDebug: @@ -67,6 +68,7 @@ static const char *MMKVLogLevelDesc(MMKVLogLevel level) { } # ifdef MMKV_IOS_OR_MAC + void _MMKVLogWithLevel(MMKVLogLevel level, const char *file, const char *func, int line, const char *format, ...) { if (level >= g_currentLogLevel) { NSString *nsFormat = [NSString stringWithUTF8String:format]; @@ -84,7 +86,9 @@ void _MMKVLogWithLevel(MMKVLogLevel level, const char *file, const char *func, i } } } + # else + void _MMKVLogWithLevel(MMKVLogLevel level, const char *file, const char *func, int line, const char *format, ...) { if (level >= g_currentLogLevel) { std::string message; @@ -116,8 +120,9 @@ void _MMKVLogWithLevel(MMKVLogLevel level, const char *file, const char *func, i } } } -# endif -# endif +# endif // MMKV_IOS_OR_MAC + +# endif // MMKV_ANDROID #endif // ENABLE_MMKV_LOG diff --git a/Core/MMKVLog_Android.cpp b/Core/MMKVLog_Android.cpp index 30a9f64a..ee2435f9 100644 --- a/Core/MMKVLog_Android.cpp +++ b/Core/MMKVLog_Android.cpp @@ -25,10 +25,13 @@ # include # include # include + using namespace std; constexpr auto APP_NAME = "MMKV"; +extern const char *_getFileName(const char *path); + static android_LogPriority MMKVLogLevelDesc(MMKVLogLevel level) { switch (level) { case MMKVLogDebug: @@ -65,10 +68,13 @@ void _MMKVLogWithLevel(MMKVLogLevel level, const char *file, const char *func, i va_end(args); } + auto filename = _getFileName(file); + if (g_logHandler) { - g_logHandler(level, file, line, func, message); + g_logHandler(level, filename, line, func, message); } else { - __android_log_print(MMKVLogLevelDesc(level), APP_NAME, "<%s:%d::%s> %s", file, line, func, message.c_str()); + auto desc = MMKVLogLevelDesc(level); + __android_log_print(desc, APP_NAME, "<%s:%d::%s> %s", filename, line, func, message.c_str()); } } } diff --git a/Core/MMKV_Android.cpp b/Core/MMKV_Android.cpp index 9d5cfdca..2dd204a2 100644 --- a/Core/MMKV_Android.cpp +++ b/Core/MMKV_Android.cpp @@ -43,7 +43,7 @@ MMKV::MMKV(const string &mmapID, int size, MMKVMode mode, string *cryptKey, stri , m_file(m_path, size, (mode & MMKV_ASHMEM) ? MMFILE_TYPE_ASHMEM : MMFILE_TYPE_FILE) , m_metaFile(m_crcPath, DEFAULT_MMAP_SIZE, m_file.m_fileType) , m_crypter(nullptr) - , m_fileLock(m_metaFile.getFd()) + , m_fileLock(m_metaFile.getFd(), (mode & MMKV_ASHMEM)) , m_sharedProcessLock(&m_fileLock, SharedLockType) , m_exclusiveProcessLock(&m_fileLock, ExclusiveLockType) , m_isInterProcess((mode & MMKV_MULTI_PROCESS) != 0 || (mode & CONTEXT_MODE_MULTI_PROCESS) != 0) { @@ -76,7 +76,7 @@ MMKV::MMKV(const string &mmapID, int ashmemFD, int ashmemMetaFD, string *cryptKe , m_file(ashmemFD) , m_metaFile(ashmemMetaFD) , m_crypter(nullptr) - , m_fileLock(m_metaFile.getFd()) + , m_fileLock(m_metaFile.getFd(), true) , m_sharedProcessLock(&m_fileLock, SharedLockType) , m_exclusiveProcessLock(&m_fileLock, ExclusiveLockType) , m_isInterProcess(true) { diff --git a/Core/PBEncodeItem.hpp b/Core/PBEncodeItem.hpp index bce27a04..ecc5a6d3 100644 --- a/Core/PBEncodeItem.hpp +++ b/Core/PBEncodeItem.hpp @@ -53,7 +53,7 @@ struct PBEncodeItem { const std::string *strValue; #else void *objectValue; - void *tmpObjectValue; // this object should release on dealloc + void *tmpObjectValue; // this object should be released on dealloc #endif } value; @@ -80,6 +80,7 @@ struct PBEncodeItem { : type(other.type), compiledSize(other.compiledSize), valueSize(other.valueSize), value(other.value) { // omit unnecessary CFRetain() & CFRelease() other.type = PBEncodeItemType_None; + //other.value = {0}; } // in fact this is never called in MMKV's usage, just provide it to meet std::vector's requirements diff --git a/Core/aes/AESCrypt.cpp b/Core/aes/AESCrypt.cpp index 402c6d6f..2754c31d 100644 --- a/Core/aes/AESCrypt.cpp +++ b/Core/aes/AESCrypt.cpp @@ -33,7 +33,7 @@ AESCrypt::AESCrypt(const void *key, size_t keyLength, const void *iv, size_t ivL if (key && keyLength > 0) { memcpy(m_key, key, (keyLength > AES_KEY_LEN) ? AES_KEY_LEN : keyLength); - reset(iv, ivLength); + resetIV(iv, ivLength); m_aesKey = new AES_KEY; memset(m_aesKey, 0, sizeof(AES_KEY)); @@ -47,7 +47,7 @@ AESCrypt::~AESCrypt() { m_aesKey = nullptr; } -void AESCrypt::reset(const void *iv, size_t ivLength) { +void AESCrypt::resetIV(const void *iv, size_t ivLength) { m_number = 0; if (iv && ivLength > 0) { memcpy(m_vector, iv, (ivLength > AES_KEY_LEN) ? AES_KEY_LEN : ivLength); diff --git a/Core/aes/AESCrypt.h b/Core/aes/AESCrypt.h index 9b891919..0ea42399 100644 --- a/Core/aes/AESCrypt.h +++ b/Core/aes/AESCrypt.h @@ -50,7 +50,7 @@ public: void decrypt(const void *input, void *output, size_t length); - void reset(const void *iv = nullptr, size_t ivLength = 0); + void resetIV(const void *iv = nullptr, size_t ivLength = 0); // output must have [AES_KEY_LEN] space void getKey(void *output) const;