From 73d1e25e023e2ef668fd162a6d2c4677a6733a34 Mon Sep 17 00:00:00 2001 From: z30053694 Date: Mon, 24 Mar 2025 11:49:31 +0800 Subject: [PATCH] feat: added interface to release client listener Signed-off-by: z30053694 Change-Id: I92a46b9f3ab3c4a6e9a7bad724e5f3281cd223bd --- frameworks/native/player/player_impl.cpp | 6 ++++ frameworks/native/player/player_impl.h | 1 + .../test/unittest/include/player_mock.h | 1 + .../player/test/unittest/src/player_mock.cpp | 6 ++++ .../test/unittest/src/player_unit_test.cpp | 30 +++++++++++++++++++ interfaces/inner_api/native/player.h | 2 ++ services/include/i_media_service.h | 5 ++++ .../services/sa_media/client/media_client.cpp | 10 +++++++ .../services/sa_media/client/media_client.h | 1 + .../sa_media/ipc/i_standard_media_service.h | 5 ++++ .../sa_media/ipc/media_service_proxy.cpp | 16 ++++++++++ .../sa_media/ipc/media_service_proxy.h | 1 + .../sa_media/ipc/media_service_stub.cpp | 16 ++++++++++ .../sa_media/ipc/media_service_stub.h | 2 ++ test/fuzztest/common/stub_common.cpp | 8 +++++ test/fuzztest/common/stub_common.h | 1 + .../playerservicestub_fuzzer.cpp | 8 +++++ 17 files changed, 119 insertions(+) diff --git a/frameworks/native/player/player_impl.cpp b/frameworks/native/player/player_impl.cpp index 5092fa451..6abbe7497 100644 --- a/frameworks/native/player/player_impl.cpp +++ b/frameworks/native/player/player_impl.cpp @@ -490,6 +490,12 @@ int32_t PlayerImpl::SetMaxAmplitudeCbStatus(bool status) return playerService_->SetMaxAmplitudeCbStatus(status); } +void PlayerImpl::ReleaseClientListener() +{ + MEDIA_LOGD("PlayerImpl:0x%{public}06" PRIXPTR " ReleaseClientListener in", FAKE_POINTER(this)); + MediaServiceFactory::GetInstance().ReleaseClientListener(); // not related to playerService_ thus no XCollie +} + PlayerImplCallback::PlayerImplCallback(const std::shared_ptr playerCb, std::shared_ptr player) { diff --git a/frameworks/native/player/player_impl.h b/frameworks/native/player/player_impl.h index cef898510..ab7ee28d0 100644 --- a/frameworks/native/player/player_impl.h +++ b/frameworks/native/player/player_impl.h @@ -79,6 +79,7 @@ public: int32_t SetMediaMuted(OHOS::Media::MediaType mediaType, bool isMuted) override; int32_t SetMaxAmplitudeCbStatus(bool status) override; int32_t SetDeviceChangeCbStatus(bool status) override; + void ReleaseClientListener() override; private: void ResetSeekVariables(); void HandleSeekDoneInfo(PlayerOnInfoType type, int32_t extra); diff --git a/frameworks/native/player/test/unittest/include/player_mock.h b/frameworks/native/player/test/unittest/include/player_mock.h index f5f4ba695..60d835159 100644 --- a/frameworks/native/player/test/unittest/include/player_mock.h +++ b/frameworks/native/player/test/unittest/include/player_mock.h @@ -161,6 +161,7 @@ public: int32_t SetMediaMuted(OHOS::Media::MediaType mediaType, bool isMuted); int32_t SetDeviceChangeCbStatus(bool status); int32_t SetMaxAmplitudeCbStatus(bool status); + void ReleaseClientListener(); private: void SeekPrepare(int32_t &mseconds, PlayerSeekMode &mode); std::shared_ptr player_ = nullptr; diff --git a/frameworks/native/player/test/unittest/src/player_mock.cpp b/frameworks/native/player/test/unittest/src/player_mock.cpp index 95a11d17c..b0d617ae3 100644 --- a/frameworks/native/player/test/unittest/src/player_mock.cpp +++ b/frameworks/native/player/test/unittest/src/player_mock.cpp @@ -770,5 +770,11 @@ int32_t PlayerMock::SetMaxAmplitudeCbStatus(bool status) UNITTEST_CHECK_AND_RETURN_RET_LOG(player_ != nullptr, -1, "player_ == nullptr"); return player_->SetMaxAmplitudeCbStatus(status); } + +void PlayerMock::ReleaseClientListener() +{ + UNITTEST_CHECK_AND_RETURN_LOG(player_ != nullptr, "player_ == nullptr"); + player_->ReleaseClientListener(); +} } // namespace Media } // namespace OHOS diff --git a/frameworks/native/player/test/unittest/src/player_unit_test.cpp b/frameworks/native/player/test/unittest/src/player_unit_test.cpp index 9ef7845d6..b0d947924 100644 --- a/frameworks/native/player/test/unittest/src/player_unit_test.cpp +++ b/frameworks/native/player/test/unittest/src/player_unit_test.cpp @@ -3577,5 +3577,35 @@ HWTEST_F(PlayerUnitTest, Player_SetMaxAmplitudeCbStatus_004, TestSize.Level0) EXPECT_EQ(MSERR_OK, player_->SetMaxAmplitudeCbStatus(false)); EXPECT_EQ(MSERR_OK, player_->Play()); } + +/** + * @tc.name : Test ReleaseClientListener API + * @tc.number: Player_ReleaseClientListener_001 + * @tc.desc : Test Player ReleaseClientListener API + */ +HWTEST_F(PlayerUnitTest, Player_ReleaseClientListener_001, TestSize.Level0) +{ + ASSERT_EQ(MSERR_OK, player_->SetSource(VIDEO_FILE1)); + sptr videoSurface = player_->GetVideoSurface(); + ASSERT_NE(nullptr, videoSurface); + EXPECT_EQ(MSERR_OK, player_->SetVideoSurface(videoSurface)); + EXPECT_EQ(MSERR_OK, player_->Prepare()); + EXPECT_EQ(MSERR_OK, player_->Play()); + + player_->ReleaseClientListener(); + EXPECT_EQ(MSERR_SERVICE_DIED, player_->SetSource(VIDEO_FILE1)); + + // re-create player + player_ = std::make_shared(callback_); + ASSERT_NE(nullptr, player_); + EXPECT_TRUE(player_->CreatePlayer()); + EXPECT_EQ(MSERR_OK, player_->SetPlayerCallback(callback_)); + ASSERT_EQ(MSERR_OK, player_->SetSource(VIDEO_FILE1)); + videoSurface = player_->GetVideoSurface(); + ASSERT_NE(nullptr, videoSurface); + EXPECT_EQ(MSERR_OK, player_->SetVideoSurface(videoSurface)); + EXPECT_EQ(MSERR_OK, player_->Prepare()); + EXPECT_EQ(MSERR_OK, player_->Play()); +} } // namespace Media } // namespace OHOS diff --git a/interfaces/inner_api/native/player.h b/interfaces/inner_api/native/player.h index 25c148b59..72066e249 100644 --- a/interfaces/inner_api/native/player.h +++ b/interfaces/inner_api/native/player.h @@ -762,6 +762,8 @@ public: return 0; } + virtual void ReleaseClientListener() = 0; + /** * @brief Enables render video first frame of the media playback. * diff --git a/services/include/i_media_service.h b/services/include/i_media_service.h index 40efcad4b..3d18517e3 100644 --- a/services/include/i_media_service.h +++ b/services/include/i_media_service.h @@ -226,6 +226,11 @@ public: * @version 1.0 */ virtual sptr GetMonitorProxy() = 0; + + /** + * @brief Release the (service-side) proxy object monitoring client aliveness + */ + virtual void ReleaseClientListener() = 0; }; class __attribute__((visibility("default"))) MediaServiceFactory { diff --git a/services/services/sa_media/client/media_client.cpp b/services/services/sa_media/client/media_client.cpp index 7237c010b..86d3cb6f4 100644 --- a/services/services/sa_media/client/media_client.cpp +++ b/services/services/sa_media/client/media_client.cpp @@ -71,6 +71,16 @@ bool MediaClient::IsAlived() return (mediaProxy_ != nullptr) ? true : false; } +void MediaClient::ReleaseClientListener() +{ + // there exist non-const methods of the sptr mediaProxy_, possible data-race. + if (mediaProxy_ == nullptr) { + return; + } + mediaProxy_->ReleaseClientListener(); + DoMediaServerDied(); // remove death recipient as well. Otherwise getting proxy after re-dlopen causes mem leak. +} + #ifdef SUPPORT_RECORDER std::shared_ptr MediaClient::CreateRecorderService() { diff --git a/services/services/sa_media/client/media_client.h b/services/services/sa_media/client/media_client.h index e5f5c5eed..45ed16a04 100644 --- a/services/services/sa_media/client/media_client.h +++ b/services/services/sa_media/client/media_client.h @@ -48,6 +48,7 @@ public: ~MediaClient(); sptr GetMonitorProxy() override; + void ReleaseClientListener() override; #ifdef SUPPORT_RECORDER std::shared_ptr CreateRecorderService() override; int32_t DestroyRecorderService(std::shared_ptr recorder) override; diff --git a/services/services/sa_media/ipc/i_standard_media_service.h b/services/services/sa_media/ipc/i_standard_media_service.h index 23b579329..90982d43e 100644 --- a/services/services/sa_media/ipc/i_standard_media_service.h +++ b/services/services/sa_media/ipc/i_standard_media_service.h @@ -51,11 +51,16 @@ public: virtual sptr GetSubSystemAbility(IStandardMediaService::MediaSystemAbility subSystemId, const sptr &listener) = 0; + /** + * Release the proxy object monitoring client process. + */ + virtual void ReleaseClientListener() = 0; /** * IPC code ID */ enum MediaServiceMsg : int32_t { GET_SUBSYSTEM = 0, + RELEASE_CLIENT_LISTENER = 2, }; DECLARE_INTERFACE_DESCRIPTOR(u"IStandardMediaService"); diff --git a/services/services/sa_media/ipc/media_service_proxy.cpp b/services/services/sa_media/ipc/media_service_proxy.cpp index 5f1127073..9ab1703b7 100644 --- a/services/services/sa_media/ipc/media_service_proxy.cpp +++ b/services/services/sa_media/ipc/media_service_proxy.cpp @@ -35,6 +35,22 @@ MediaServiceProxy::~MediaServiceProxy() MEDIA_LOGD("0x%{public}06" PRIXPTR " Instances destroy", FAKE_POINTER(this)); } +void MediaServiceProxy::ReleaseClientListener() +{ + MessageParcel data; + MessageParcel reply; + MessageOption option; + + bool token = data.WriteInterfaceToken(MediaServiceProxy::GetDescriptor()); + CHECK_AND_RETURN_LOG(token, "Failed to write descriptor!"); + + int32_t error = -1; + error = Remote()->SendRequest(MediaServiceMsg::RELEASE_CLIENT_LISTENER, data, reply, option); + if (error != MSERR_OK) { + MEDIA_LOGE("Send request failed, error: %{public}d", error); + } +} + sptr MediaServiceProxy::GetSubSystemAbility(IStandardMediaService::MediaSystemAbility subSystemId, const sptr &listener) { diff --git a/services/services/sa_media/ipc/media_service_proxy.h b/services/services/sa_media/ipc/media_service_proxy.h index 5c96f3404..52aca5717 100644 --- a/services/services/sa_media/ipc/media_service_proxy.h +++ b/services/services/sa_media/ipc/media_service_proxy.h @@ -28,6 +28,7 @@ public: sptr GetSubSystemAbility(IStandardMediaService::MediaSystemAbility subSystemId, const sptr &listener) override; + void ReleaseClientListener() override; private: static inline BrokerDelegator delegator_; diff --git a/services/services/sa_media/ipc/media_service_stub.cpp b/services/services/sa_media/ipc/media_service_stub.cpp index 9b9f86706..68403827b 100644 --- a/services/services/sa_media/ipc/media_service_stub.cpp +++ b/services/services/sa_media/ipc/media_service_stub.cpp @@ -43,6 +43,8 @@ void MediaServiceStub::Init() { mediaFuncs_[GET_SUBSYSTEM] = [this](MessageParcel &data, MessageParcel &reply) { return GetSystemAbility(data, reply); }; + mediaFuncs_[RELEASE_CLIENT_LISTENER] = + [this] (MessageParcel &data, MessageParcel &reply) { return ReleaseClientListenerStub(data, reply); }; } int32_t MediaServiceStub::DestroyStubForPid(pid_t pid) @@ -144,6 +146,20 @@ int32_t MediaServiceStub::SetDeathListener(const sptr &object) return MSERR_OK; } +int32_t MediaServiceStub::ReleaseClientListenerStub(MessageParcel &data, MessageParcel &reply) +{ + (void)data; + (void)reply; + ReleaseClientListener(); + return MSERR_OK; +} + +void MediaServiceStub::ReleaseClientListener() +{ + pid_t pid = IPCSkeleton::GetCallingPid(); + (void)DestroyStubForPid(pid); +} + int32_t MediaServiceStub::GetSystemAbility(MessageParcel &data, MessageParcel &reply) { int32_t mediaSystemAbility = data.ReadInt32(); diff --git a/services/services/sa_media/ipc/media_service_stub.h b/services/services/sa_media/ipc/media_service_stub.h index e19a54da3..391418b97 100644 --- a/services/services/sa_media/ipc/media_service_stub.h +++ b/services/services/sa_media/ipc/media_service_stub.h @@ -38,6 +38,8 @@ protected: private: void Init(); int32_t GetSystemAbility(MessageParcel &data, MessageParcel &reply); + int32_t ReleaseClientListenerStub(MessageParcel &data, MessageParcel &reply); + void ReleaseClientListener() override; void ClientDied(pid_t pid); int32_t DestroyStubForPid(pid_t pid); std::map mediaFuncs_; diff --git a/test/fuzztest/common/stub_common.cpp b/test/fuzztest/common/stub_common.cpp index 7d5501596..3ce4e30b2 100644 --- a/test/fuzztest/common/stub_common.cpp +++ b/test/fuzztest/common/stub_common.cpp @@ -38,5 +38,13 @@ sptr MediaServiceProxyFuzzer::GetSubSystemAbility(IStandardMediaS } return reply.ReadRemoteObject(); } + +void MediaServiceProxyFuzzer::ReleaseClientListener() +{ + MessageParcel data; + MessageParcel reply; + MessageOption option; + (void)Remote()->SendRequest(MediaServiceMsg::RELEASE_CLIENT_LISTENER, data, reply, option); +} } } \ No newline at end of file diff --git a/test/fuzztest/common/stub_common.h b/test/fuzztest/common/stub_common.h index cb3bc0301..185b137e8 100644 --- a/test/fuzztest/common/stub_common.h +++ b/test/fuzztest/common/stub_common.h @@ -35,6 +35,7 @@ public: virtual ~MediaServiceProxyFuzzer() {} sptr GetSubSystemAbility(IStandardMediaService::MediaSystemAbility subSystemId, const sptr &listener); + void ReleaseClientListener(); private: static inline BrokerDelegator delegator_; }; diff --git a/test/fuzztest/player_fuzztest/playerservicestub_fuzzer/playerservicestub_fuzzer.cpp b/test/fuzztest/player_fuzztest/playerservicestub_fuzzer/playerservicestub_fuzzer.cpp index de0d38088..b8c0df505 100644 --- a/test/fuzztest/player_fuzztest/playerservicestub_fuzzer/playerservicestub_fuzzer.cpp +++ b/test/fuzztest/player_fuzztest/playerservicestub_fuzzer/playerservicestub_fuzzer.cpp @@ -55,6 +55,14 @@ public: return reply.ReadRemoteObject(); } + void ReleaseClientListener() + { + MessageParcel data; + MessageParcel reply; + MessageOption option; + (void)Remote()->SendRequest(MediaServiceMsg::RELEASE_CLIENT_LISTENER, data, reply, option); + } + private: static inline BrokerDelegator delegator_; }; -- Gitee