Skip to content

Commit

Permalink
Fixed buggy update_usock (#919)
Browse files Browse the repository at this point in the history
* Fixed buggy update_usock.
  • Loading branch information
ethouris authored and rndi committed Oct 29, 2019
1 parent 4f72a51 commit 0f8e93e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 32 deletions.
38 changes: 30 additions & 8 deletions srtcore/epoll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,22 +233,44 @@ int CEPoll::update_usock(const int eid, const SRTSOCKET& u, const int* events)
if (p == m_mPolls.end())
throw CUDTException(MJ_NOTSUP, MN_EIDINVAL);

CEPollDesc& d = p->second;

int32_t evts = events ? *events : uint32_t(SRT_EPOLL_IN | SRT_EPOLL_OUT | SRT_EPOLL_ERR);
bool edgeTriggered = evts & SRT_EPOLL_ET;
evts &= ~SRT_EPOLL_ET;
if (evts)
{
pair<CEPollDesc::ewatch_t::iterator, bool> iter_new = p->second.addWatch(u, evts, edgeTriggered);
pair<CEPollDesc::ewatch_t::iterator, bool> iter_new = d.addWatch(u, evts, edgeTriggered);
CEPollDesc::Wait& wait = iter_new.first->second;
int newstate = wait.watch & wait.state;
if (newstate)
if (!iter_new.second)
{
p->second.addEventNotice(wait, u, newstate);
// The object exists. We only are certain about the `u`
// parameter, but others are probably unchanged. Change them
// forcefully and take out notices that are no longer valid.
const int removable = wait.watch & ~evts;

// Check if there are any events that would be removed.
// If there are no removed events watched (for example, when
// only new events are being added to existing socket),
// there's nothing to remove, but might be something to update.
if (removable)
{
d.removeExcessEvents(wait, evts);
}

// Update the watch configuration, including edge
wait.watch = evts;
if (edgeTriggered)
wait.edge = evts;

// Now it should look exactly like newly added
// and the state is also updated
}
else if (!iter_new.second) // if it was freshly added, no notice object exists

const int newstate = wait.watch & wait.state;
if (newstate)
{
// This removes the event notice entry, but leaves the subscription
p->second.removeEvents(wait);
d.addEventNotice(wait, u, newstate);
}
}
else if (edgeTriggered)
Expand All @@ -259,7 +281,7 @@ int CEPoll::update_usock(const int eid, const SRTSOCKET& u, const int* events)
else
{
// Update with no events means to remove subscription
p->second.removeSubscription(u);
d.removeSubscription(u);
}
return 0;
}
Expand Down
56 changes: 32 additions & 24 deletions srtcore/epoll.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,30 +205,7 @@ struct CEPollDesc
}
else
{
// `events` contains bits to be cleared.
// 1. If there is no notice event, do nothing - clear already.
// 2. If there is a notice event, update by clearing the bits
// 2.1. If this made resulting state to be 0, also remove the notice.

// If wait.notit is empty, there's no event to clear
if (wait.notit == nullNotice())
return;

// Update the state
const int newstate = wait.notit->events & (~events);

if (newstate == 0)
{
// If the new state is full 0 (no events),
// then remove the corresponding notice object
m_USockEventNotice.erase(wait.notit);

// and set the "corresponding notice object" to nothing
wait.notit = nullNotice();
return;
}

wait.notit->events = newstate;
removeExcessEvents(wait, ~events);
}
}

Expand Down Expand Up @@ -260,6 +237,37 @@ struct CEPollDesc
removeExistingNotices(wait);
}

// This function removes notices referring to
// events that are NOT present in @a nevts, but
// may be among subscriptions and therefore potentially
// have an associated notice.
void removeExcessEvents(Wait& wait, int nevts)
{
// Update the event notice, should it exist
// If the watch points to a null notice, there's simply
// no notice there, so nothing to update or prospectively
// remove - but may be something to add.
if (wait.notit == nullNotice())
return;

// `events` contains bits to be cleared.
// 1. If there is no notice event, do nothing - clear already.
// 2. If there is a notice event, update by clearing the bits
// 2.1. If this made resulting state to be 0, also remove the notice.

const int newstate = wait.notit->events & nevts;
if (newstate)
{
wait.notit->events = newstate;
}
else
{
// If the new state is full 0 (no events),
// then remove the corresponding notice object
removeExistingNotices(wait);
}
}

void checkEdge(enotice_t::iterator i)
{
// This function should check if this event was subscribed
Expand Down

0 comments on commit 0f8e93e

Please sign in to comment.