Skip to content

Commit

Permalink
Do not call uv_close from uv loop other than the main one (#1443)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcague
jcague authored Jul 31, 2019
1 parent 28d4f90 commit 67f9102
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 32 deletions.
58 changes: 27 additions & 31 deletions erizoAPI/MediaStream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<uv_handle_t*>(async_stats_))) {
Expand All @@ -78,9 +78,6 @@ void MediaStream::closeEvents() {
uv_close(reinterpret_cast<uv_handle_t*>(async_event_), destroyAsyncHandle);
}
async_event_ = nullptr;
}

void MediaStream::closeFutureAsync() {
if (!uv_is_closing(reinterpret_cast<uv_handle_t*>(close_future_async_))) {
ELOG_DEBUG("%s, message: Closing future handle", toLog());
uv_close(reinterpret_cast<uv_handle_t*>(close_future_async_), destroyAsyncHandle);
Expand All @@ -97,18 +94,17 @@ boost::future<void> 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<void>) {
closeEvents();
me.reset();
close_promise->set_value();
});
} else {
closeEvents();
closed_ = true;
close_promise->set_value();
}
ELOG_DEBUG("%s, message: Closed", toLog());
Expand Down Expand Up @@ -204,7 +200,7 @@ NAN_METHOD(MediaStream::close) {
NAN_METHOD(MediaStream::init) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}
bool force = info.Length() > 0 ? info[0]->BooleanValue() : false;
Expand All @@ -216,7 +212,7 @@ NAN_METHOD(MediaStream::init) {
NAN_METHOD(MediaStream::setSlideShowMode) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -228,7 +224,7 @@ NAN_METHOD(MediaStream::setSlideShowMode) {
NAN_METHOD(MediaStream::muteStream) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -240,7 +236,7 @@ NAN_METHOD(MediaStream::muteStream) {
NAN_METHOD(MediaStream::setMaxVideoBW) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -251,7 +247,7 @@ NAN_METHOD(MediaStream::setMaxVideoBW) {
NAN_METHOD(MediaStream::setVideoConstraints) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}
int max_video_width = info[0]->IntegerValue();
Expand All @@ -263,7 +259,7 @@ NAN_METHOD(MediaStream::setVideoConstraints) {
NAN_METHOD(MediaStream::setMetadata) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand Down Expand Up @@ -291,7 +287,7 @@ NAN_METHOD(MediaStream::setMetadata) {
NAN_METHOD(MediaStream::getCurrentState) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -304,7 +300,7 @@ NAN_METHOD(MediaStream::getCurrentState) {
NAN_METHOD(MediaStream::setAudioReceiver) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -318,7 +314,7 @@ NAN_METHOD(MediaStream::setAudioReceiver) {
NAN_METHOD(MediaStream::setVideoReceiver) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -334,7 +330,7 @@ NAN_METHOD(MediaStream::generatePLIPacket) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());

std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}
me->sendPLI();
Expand All @@ -343,7 +339,7 @@ NAN_METHOD(MediaStream::generatePLIPacket) {
NAN_METHOD(MediaStream::enableHandler) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -357,7 +353,7 @@ NAN_METHOD(MediaStream::enableHandler) {
NAN_METHOD(MediaStream::disableHandler) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -370,7 +366,7 @@ NAN_METHOD(MediaStream::disableHandler) {
NAN_METHOD(MediaStream::setQualityLayer) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -383,7 +379,7 @@ NAN_METHOD(MediaStream::setQualityLayer) {
NAN_METHOD(MediaStream::enableSlideShowBelowSpatialLayer) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -394,7 +390,7 @@ NAN_METHOD(MediaStream::enableSlideShowBelowSpatialLayer) {

NAN_METHOD(MediaStream::getStats) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(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<Function>());
Expand All @@ -403,7 +399,7 @@ NAN_METHOD(MediaStream::getStats) {

NAN_METHOD(MediaStream::getPeriodicStats) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
if (obj->me == nullptr || info.Length() != 1) {
if (!obj->me || info.Length() != 1 || obj->closed_) {
return;
}
obj->me->setMediaStreamStatsListener(obj);
Expand All @@ -414,7 +410,7 @@ NAN_METHOD(MediaStream::getPeriodicStats) {
NAN_METHOD(MediaStream::setFeedbackReports) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}

Expand All @@ -426,7 +422,7 @@ NAN_METHOD(MediaStream::setFeedbackReports) {
NAN_METHOD(MediaStream::onMediaStreamEvent) {
MediaStream* obj = Nan::ObjectWrap::Unwrap<MediaStream>(info.Holder());
std::shared_ptr<erizo::MediaStream> me = obj->me;
if (!me) {
if (!me || obj->closed_) {
return;
}
me ->setMediaStreamEventListener(obj);
Expand Down Expand Up @@ -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<MediaStream*>(async->data);
if (!obj || !obj->me) {
if (!obj || !obj->me || obj->closed_) {
return;
}
boost::mutex::scoped_lock lock(obj->mutex);
Expand All @@ -480,7 +476,7 @@ NAUV_WORK_CB(MediaStream::statsCallback) {
NAUV_WORK_CB(MediaStream::eventCallback) {
Nan::HandleScope scope;
MediaStream* obj = reinterpret_cast<MediaStream*>(async->data);
if (!obj || !obj->me) {
if (!obj || !obj->me || obj->closed_) {
return;
}
boost::mutex::scoped_lock lock(obj->mutex);
Expand Down Expand Up @@ -511,7 +507,7 @@ void MediaStream::notifyFuture(Nan::Persistent<v8::Promise::Resolver> *persisten
NAUV_WORK_CB(MediaStream::closePromiseResolver) {
Nan::HandleScope scope;
MediaStream* obj = reinterpret_cast<MediaStream*>(async->data);
if (!obj) {
if (!obj) { // closed_ will always be true here
return;
}
boost::mutex::scoped_lock lock(obj->mutex);
Expand All @@ -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());
}
1 change: 0 additions & 1 deletion erizoAPI/MediaStream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class MediaStream : public MediaSink, public erizo::MediaStreamStatsListener, pu

boost::future<void> close();
void closeEvents();
void closeFutureAsync();
std::string toLog();

Nan::Callback *event_callback_;
Expand Down

0 comments on commit 67f9102

Please sign in to comment.