From c99839aa66059ace0a19ed11bcb2ecaa95300bde Mon Sep 17 00:00:00 2001 From: peilixia Date: Thu, 27 Oct 2022 11:57:14 +0000 Subject: [PATCH 1/4] fixed 494fd97 from https://gitee.com/peilixia/commonlibrary_c_utils/pulls/135 ICSL modify Signed-off-by: peilixia --- base/src/event_reactor.cpp | 3 --- base/src/parcel.cpp | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/base/src/event_reactor.cpp b/base/src/event_reactor.cpp index e54d183..ea53837 100644 --- a/base/src/event_reactor.cpp +++ b/base/src/event_reactor.cpp @@ -95,9 +95,6 @@ uint32_t EventReactor::ScheduleTimer(const TimerCallback& cb, uint32_t interval, { std::lock_guard lock(mutex_); std::shared_ptr handler = std::make_shared(this, interval, once); - if (handler == nullptr) { - return TIMER_ERR_INVALID_VALUE; - } handler->SetTimerCallback(cb); uint32_t ret = handler->Initialize(); if (ret != TIMER_ERR_OK) { diff --git a/base/src/parcel.cpp b/base/src/parcel.cpp index ae218ee..441a4df 100644 --- a/base/src/parcel.cpp +++ b/base/src/parcel.cpp @@ -964,8 +964,8 @@ const std::string Parcel::ReadString() return std::string(); } - size_t readCapacity = dataLength + 1; - if ((readCapacity > (size_t)dataLength) && (readCapacity <= GetReadableBytes())) { + size_t readCapacity = static_cast(dataLength) + 1; + if (readCapacity <= GetReadableBytes()) { const uint8_t *dest = ReadBuffer(readCapacity); if (dest != nullptr) { const auto *str = reinterpret_cast(dest); @@ -990,8 +990,8 @@ bool Parcel::ReadString(std::string &value) return false; } - size_t readCapacity = dataLength + 1; - if ((readCapacity > (size_t)dataLength) && (readCapacity <= GetReadableBytes())) { + size_t readCapacity = static_cast(dataLength) + 1; + if (readCapacity <= GetReadableBytes()) { const uint8_t *dest = ReadBuffer(readCapacity); if (dest != nullptr) { const auto *str = reinterpret_cast(dest); @@ -1017,8 +1017,8 @@ const std::u16string Parcel::ReadString16() return std::u16string(); } - size_t readCapacity = (dataLength + 1) * sizeof(char16_t); - if ((readCapacity > (size_t)dataLength) && (readCapacity <= GetReadableBytes())) { + size_t readCapacity = (static_cast(dataLength) + 1) * sizeof(char16_t); + if ((readCapacity > (static_cast(dataLength))) && (readCapacity <= GetReadableBytes())) { const uint8_t *str = ReadBuffer(readCapacity); if (str != nullptr) { const auto *u16Str = reinterpret_cast(str); @@ -1043,7 +1043,7 @@ bool Parcel::ReadString16(std::u16string &value) return false; } - size_t readCapacity = (dataLength + 1) * sizeof(char16_t); + size_t readCapacity = (static_cast(dataLength) + 1) * sizeof(char16_t); if (readCapacity <= GetReadableBytes()) { const uint8_t *str = ReadBuffer(readCapacity); if (str != nullptr) { @@ -1075,8 +1075,8 @@ const std::u16string Parcel::ReadString16WithLength(int32_t &readLength) return std::u16string(); } - size_t readCapacity = (dataLength + 1) * sizeof(char16_t); - if ((readCapacity > (size_t)dataLength) && (readCapacity <= GetReadableBytes())) { + size_t readCapacity = (static_cast(dataLength) + 1) * sizeof(char16_t); + if ((readCapacity > (static_cast(dataLength))) && (readCapacity <= GetReadableBytes())) { const uint8_t *str = ReadBuffer(readCapacity); if (str != nullptr) { const auto *u16Str = reinterpret_cast(str); @@ -1106,8 +1106,8 @@ const std::string Parcel::ReadString8WithLength(int32_t &readLength) return std::string(); } - size_t readCapacity = (dataLength + 1) * sizeof(char); - if ((readCapacity > (size_t)dataLength) && (readCapacity <= GetReadableBytes())) { + size_t readCapacity = (static_cast(dataLength) + 1) * sizeof(char); + if (readCapacity <= GetReadableBytes()) { const uint8_t *str = ReadBuffer(readCapacity); if (str != nullptr) { const auto *u8Str = reinterpret_cast(str); -- Gitee From 366911ac01f994b27ce82614a77c30d2e535cac0 Mon Sep 17 00:00:00 2001 From: fanxiaoyu Date: Tue, 15 Nov 2022 02:35:58 +0000 Subject: [PATCH 2/4] Add GetRefCounter interface Signed-off-by: fanxiaoyu --- base/include/refbase.h | 2 ++ base/src/refbase.cpp | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/base/include/refbase.h b/base/include/refbase.h index 8122d81..e7df34a 100644 --- a/base/include/refbase.h +++ b/base/include/refbase.h @@ -188,6 +188,8 @@ public: WeakRefCounter *CreateWeakRef(void *cookie); + RefCounter *GetRefCounter() const; + void IncWeakRef(const void *objectId); void DecWeakRef(const void *objectId); diff --git a/base/src/refbase.cpp b/base/src/refbase.cpp index fb46127..ad5665e 100644 --- a/base/src/refbase.cpp +++ b/base/src/refbase.cpp @@ -361,7 +361,7 @@ bool RefCounter::AttemptIncStrong(const void *objectId) if (atomicStrong_.compare_exchange_weak(curCount, curCount + 1, std::memory_order_relaxed)) { break; } - curCount = atomicStrong_.load(std::memory_order_relaxed); + // curCount has been updated. } if (curCount <= 0) { DecWeakRefCount(objectId); @@ -511,6 +511,11 @@ void RefBase::IncWeakRef(const void *objectId) } } +RefCounter *RefBase::GetRefCounter() const +{ + return refs_; +} + void RefBase::DecWeakRef(const void *objectId) { if (refs_ != nullptr) { -- Gitee From b80f85167448bf4769e30e9ad4f00477b1069b51 Mon Sep 17 00:00:00 2001 From: fanxiaoyu Date: Tue, 22 Nov 2022 00:58:49 +0000 Subject: [PATCH 3/4] fixed 15a44a7 from https://gitee.com/shufewhx/commonlibrary_c_utils/pulls/144 Revert new interface Signed-off-by: fanxiaoyu --- base/include/refbase.h | 2 -- base/src/refbase.cpp | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/base/include/refbase.h b/base/include/refbase.h index e7df34a..8122d81 100644 --- a/base/include/refbase.h +++ b/base/include/refbase.h @@ -188,8 +188,6 @@ public: WeakRefCounter *CreateWeakRef(void *cookie); - RefCounter *GetRefCounter() const; - void IncWeakRef(const void *objectId); void DecWeakRef(const void *objectId); diff --git a/base/src/refbase.cpp b/base/src/refbase.cpp index ad5665e..fb46127 100644 --- a/base/src/refbase.cpp +++ b/base/src/refbase.cpp @@ -361,7 +361,7 @@ bool RefCounter::AttemptIncStrong(const void *objectId) if (atomicStrong_.compare_exchange_weak(curCount, curCount + 1, std::memory_order_relaxed)) { break; } - // curCount has been updated. + curCount = atomicStrong_.load(std::memory_order_relaxed); } if (curCount <= 0) { DecWeakRefCount(objectId); @@ -511,11 +511,6 @@ void RefBase::IncWeakRef(const void *objectId) } } -RefCounter *RefBase::GetRefCounter() const -{ - return refs_; -} - void RefBase::DecWeakRef(const void *objectId) { if (refs_ != nullptr) { -- Gitee From a5180fcac14331a3195b8739f2d29c55447773ec Mon Sep 17 00:00:00 2001 From: liujialiang Date: Sat, 26 Nov 2022 12:58:54 +0800 Subject: [PATCH 4/4] Add test suit for RWLock RWLock is a part of libutils.z.so, but never has a test suit. So we add one for it. We prepare two test cases for RWLock to verify that it works well under two different mode(write-first or not). Signed-off-by: liujialiang --- base/test/unittest/common/BUILD.gn | 14 ++ .../unittest/common/utils_rwlock_test.cpp | 128 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 base/test/unittest/common/utils_rwlock_test.cpp diff --git a/base/test/unittest/common/BUILD.gn b/base/test/unittest/common/BUILD.gn index 0d7f4b4..f9f6cba 100644 --- a/base/test/unittest/common/BUILD.gn +++ b/base/test/unittest/common/BUILD.gn @@ -292,6 +292,19 @@ ohos_unittest("UtilsTimerTest") { ] } +############################################################################### +ohos_unittest("UtilsRWLockTest") { + module_out_path = module_output_path + sources = [ "utils_rwlock_test.cpp" ] + + configs = [ ":module_private_config" ] + + deps = [ + "//commonlibrary/c_utils/base:utils", + "//third_party/googletest:gtest_main", + ] +} + ############################################################################### group("unittest") { @@ -304,6 +317,7 @@ group("unittest") { ":UtilsDateTimeTest", ":UtilsDirectoryTest", ":UtilsParcelTest", + ":UtilsRWLockTest", ":UtilsRefbaseTest", ":UtilsSafeBlockQueueTest", ":UtilsSafeBlockQueueTrackingTest", diff --git a/base/test/unittest/common/utils_rwlock_test.cpp b/base/test/unittest/common/utils_rwlock_test.cpp new file mode 100644 index 0000000..dbd6d48 --- /dev/null +++ b/base/test/unittest/common/utils_rwlock_test.cpp @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2021 Huawei Device Co., Ltd. + * 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. + */ +#include +#include +#include + +#include "rwlock.h" + +using namespace testing::ext; +using namespace std; + +namespace OHOS { +namespace { + +class UtilsRWLockTest : public testing::Test {}; + +// This class is designed for test RWLock. "buf" is protected by "rwLock". +class TestRWLock { +public: + TestRWLock():rwLock(), buf() {} + + explicit TestRWLock(bool writeFirst):rwLock(writeFirst), buf() {} + + void WriteStr(const string& str) + { + rwLock.LockWrite(); + for (auto it = str.begin(); it != str.end(); it++) { + buf.push_back(*it); + this_thread::sleep_for(std::chrono::milliseconds(1)); + } + rwLock.UnLockWrite(); + return; + } + + void ReadStr(string& str) + { + rwLock.LockRead(); + for (auto it = buf.begin(); it != buf.end(); it++) { + str.push_back(*it); + this_thread::sleep_for(std::chrono::milliseconds(1)); + } + rwLock.UnLockRead(); + return; + } +private: + Utils::RWLock rwLock; + string buf; +}; + +const string WRITE_IN_1("write1"); +const string WRITE_IN_2("write2"); + +/* + * @tc.name: testRWLock001 + * @tc.desc: RWLock here is under write-first mode. If there are some writing operation waiting, + * reading will never happen. Reading operations will happen at the same time, when all writing operations + * have finished. + */ +HWTEST_F(UtilsRWLockTest, testRWLock001, TestSize.Level1) +{ + TestRWLock test; + + thread first(bind(&TestRWLock::WriteStr, ref(test), ref(WRITE_IN_1))); + this_thread::sleep_for(chrono::milliseconds(1)); + + string readOut1(""); + thread second(bind(&TestRWLock::ReadStr, ref(test), ref(readOut1))); + this_thread::sleep_for(chrono::milliseconds(1)); + + thread third(bind(&TestRWLock::WriteStr, ref(test), ref(WRITE_IN_2))); + this_thread::sleep_for(chrono::milliseconds(1)); + + string readOut2(""); + thread fourth(bind(&TestRWLock::ReadStr, ref(test), ref(readOut2))); + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ(readOut1, WRITE_IN_1 + WRITE_IN_2); + EXPECT_EQ(readOut2, WRITE_IN_1 + WRITE_IN_2); +} + +/* + * @tc.name: testRWLock002 + * @tc.desc: RWLock here is not under write-first mode. So if there are writing and reading operations in queue + * with a writing mission running, they will compete when the writing mission completing. But what we can ensure + * is that reading operations in queue will happen at the same time. + */ +HWTEST_F(UtilsRWLockTest, testRWLock002, TestSize.Level1) +{ + TestRWLock test(false); + + thread first(bind(&TestRWLock::WriteStr, ref(test), ref(WRITE_IN_1))); + this_thread::sleep_for(chrono::milliseconds(1)); + + string readOut1(""); + thread second(bind(&TestRWLock::ReadStr, ref(test), ref(readOut1))); + this_thread::sleep_for(chrono::milliseconds(1)); + + thread third(bind(&TestRWLock::WriteStr, ref(test), ref(WRITE_IN_2))); + this_thread::sleep_for(chrono::milliseconds(1)); + + string readOut2(""); + thread fourth(bind(&TestRWLock::ReadStr, ref(test), ref(readOut2))); + + first.join(); + second.join(); + third.join(); + fourth.join(); + + EXPECT_EQ(readOut1, readOut2); +} +} // namespace +} // namespace OHOS \ No newline at end of file -- Gitee