From 67f91023f61bc0e47de188857189feb1077eadbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Cervi=C3=B1o?= Date: Wed, 31 Jul 2019 15:01:11 +0200 Subject: [PATCH] Do not call uv_close from uv loop other than the main one (#1443) --- erizoAPI/MediaStream.cc | 58 +++++++++++++++++++---------------------- erizoAPI/MediaStream.h | 1 - 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/erizoAPI/MediaStream.cc b/erizoAPI/MediaStream.cc index f8301766d0..5a14ef87b0 100644 --- a/erizoAPI/MediaStream.cc +++ b/erizoAPI/MediaStream.cc @@ -60,12 +60,12 @@ MediaStream::MediaStream() : closed_{false}, id_{"undefined"} { } MediaStream::~MediaStream() { - close(); + boost::mutex::scoped_lock lock(mutex); + closeEvents(); ELOG_DEBUG("%s, message: Destroyed", toLog()); } void MediaStream::closeEvents() { - boost::mutex::scoped_lock lock(mutex); has_stats_callback_ = false; has_event_callback_ = false; if (!uv_is_closing(reinterpret_cast(async_stats_))) { @@ -78,9 +78,6 @@ void MediaStream::closeEvents() { uv_close(reinterpret_cast(async_event_), destroyAsyncHandle); } async_event_ = nullptr; -} - -void MediaStream::closeFutureAsync() { if (!uv_is_closing(reinterpret_cast(close_future_async_))) { ELOG_DEBUG("%s, message: Closing future handle", toLog()); uv_close(reinterpret_cast(close_future_async_), destroyAsyncHandle); @@ -97,18 +94,17 @@ boost::future MediaStream::close() { return close_promise->get_future(); } ELOG_DEBUG("%s, message: Closing", toLog()); + closed_ = true; + if (me) { me->setMediaStreamStatsListener(nullptr); me->setMediaStreamEventListener(nullptr); - closed_ = true; + me->close().then([this, close_promise] (boost::future) { - closeEvents(); me.reset(); close_promise->set_value(); }); } else { - closeEvents(); - closed_ = true; close_promise->set_value(); } ELOG_DEBUG("%s, message: Closed", toLog()); @@ -204,7 +200,7 @@ NAN_METHOD(MediaStream::close) { NAN_METHOD(MediaStream::init) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } bool force = info.Length() > 0 ? info[0]->BooleanValue() : false; @@ -216,7 +212,7 @@ NAN_METHOD(MediaStream::init) { NAN_METHOD(MediaStream::setSlideShowMode) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -228,7 +224,7 @@ NAN_METHOD(MediaStream::setSlideShowMode) { NAN_METHOD(MediaStream::muteStream) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -240,7 +236,7 @@ NAN_METHOD(MediaStream::muteStream) { NAN_METHOD(MediaStream::setMaxVideoBW) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -251,7 +247,7 @@ NAN_METHOD(MediaStream::setMaxVideoBW) { NAN_METHOD(MediaStream::setVideoConstraints) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } int max_video_width = info[0]->IntegerValue(); @@ -263,7 +259,7 @@ NAN_METHOD(MediaStream::setVideoConstraints) { NAN_METHOD(MediaStream::setMetadata) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -291,7 +287,7 @@ NAN_METHOD(MediaStream::setMetadata) { NAN_METHOD(MediaStream::getCurrentState) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -304,7 +300,7 @@ NAN_METHOD(MediaStream::getCurrentState) { NAN_METHOD(MediaStream::setAudioReceiver) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -318,7 +314,7 @@ NAN_METHOD(MediaStream::setAudioReceiver) { NAN_METHOD(MediaStream::setVideoReceiver) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -334,7 +330,7 @@ NAN_METHOD(MediaStream::generatePLIPacket) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } me->sendPLI(); @@ -343,7 +339,7 @@ NAN_METHOD(MediaStream::generatePLIPacket) { NAN_METHOD(MediaStream::enableHandler) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -357,7 +353,7 @@ NAN_METHOD(MediaStream::enableHandler) { NAN_METHOD(MediaStream::disableHandler) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -370,7 +366,7 @@ NAN_METHOD(MediaStream::disableHandler) { NAN_METHOD(MediaStream::setQualityLayer) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -383,7 +379,7 @@ NAN_METHOD(MediaStream::setQualityLayer) { NAN_METHOD(MediaStream::enableSlideShowBelowSpatialLayer) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -394,7 +390,7 @@ NAN_METHOD(MediaStream::enableSlideShowBelowSpatialLayer) { NAN_METHOD(MediaStream::getStats) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); - if (!obj->me || info.Length() != 1) { + if (!obj->me || info.Length() != 1 || obj->closed_) { return; } Nan::Callback *callback = new Nan::Callback(info[0].As()); @@ -403,7 +399,7 @@ NAN_METHOD(MediaStream::getStats) { NAN_METHOD(MediaStream::getPeriodicStats) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); - if (obj->me == nullptr || info.Length() != 1) { + if (!obj->me || info.Length() != 1 || obj->closed_) { return; } obj->me->setMediaStreamStatsListener(obj); @@ -414,7 +410,7 @@ NAN_METHOD(MediaStream::getPeriodicStats) { NAN_METHOD(MediaStream::setFeedbackReports) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } @@ -426,7 +422,7 @@ NAN_METHOD(MediaStream::setFeedbackReports) { NAN_METHOD(MediaStream::onMediaStreamEvent) { MediaStream* obj = Nan::ObjectWrap::Unwrap(info.Holder()); std::shared_ptr me = obj->me; - if (!me) { + if (!me || obj->closed_) { return; } me ->setMediaStreamEventListener(obj); @@ -463,7 +459,7 @@ void MediaStream::notifyMediaStreamEvent(const std::string& type, const std::str NAUV_WORK_CB(MediaStream::statsCallback) { Nan::HandleScope scope; MediaStream* obj = reinterpret_cast(async->data); - if (!obj || !obj->me) { + if (!obj || !obj->me || obj->closed_) { return; } boost::mutex::scoped_lock lock(obj->mutex); @@ -480,7 +476,7 @@ NAUV_WORK_CB(MediaStream::statsCallback) { NAUV_WORK_CB(MediaStream::eventCallback) { Nan::HandleScope scope; MediaStream* obj = reinterpret_cast(async->data); - if (!obj || !obj->me) { + if (!obj || !obj->me || obj->closed_) { return; } boost::mutex::scoped_lock lock(obj->mutex); @@ -511,7 +507,7 @@ void MediaStream::notifyFuture(Nan::Persistent *persisten NAUV_WORK_CB(MediaStream::closePromiseResolver) { Nan::HandleScope scope; MediaStream* obj = reinterpret_cast(async->data); - if (!obj) { + if (!obj) { // closed_ will always be true here return; } boost::mutex::scoped_lock lock(obj->mutex); @@ -525,7 +521,7 @@ NAUV_WORK_CB(MediaStream::closePromiseResolver) { obj->futures.pop(); obj->Unref(); } - obj->closeFutureAsync(); + obj->closeEvents(); obj->Unref(); ELOG_DEBUG("%s, message: closePromiseResolver finished", obj->toLog()); } diff --git a/erizoAPI/MediaStream.h b/erizoAPI/MediaStream.h index 7d9d2defcf..3bd841e2b2 100644 --- a/erizoAPI/MediaStream.h +++ b/erizoAPI/MediaStream.h @@ -51,7 +51,6 @@ class MediaStream : public MediaSink, public erizo::MediaStreamStatsListener, pu boost::future close(); void closeEvents(); - void closeFutureAsync(); std::string toLog(); Nan::Callback *event_callback_;