fix epoll event loss problem

[cg82616424]

more detailed informations:

1. user call receive/receivemessage(about line number:6482)
2. after read/receive, if rcvbuffer is empty, will set SRT_EPOLL_IN event to false
3. but if we do not do some lock work here, will cause some sync problems between threads:
   (1) user thread: call receive/receivemessage
   (2) user thread: read data
   (3) user thread: no data in rcvbuffer, set SRT_EPOLL_IN event to false
   (4) receive thread: receive data and set SRT_EPOLL_IN to true
   (5) user thread: set SRT_EPOLL_IN to false
4. so , m_RecvLock must be used here to protect epoll event

[cg82616424]

I viewed my code, I think continuous-integration/travis-ci/pr should be retry to solve this error, what should i do ?

[cg82616424]

I viewed my code, I think continuous-integration/travis-ci/pr should be retry to solve this error, what should i do ?

I retied , all successed

[ethouris]

Just one more thing before we start seeing to it: Did you read the LowLevelInfo.md document? It describes the already found and confirmed mutex ordering. Before you change anything in the mutex locking, make sure that the ordering there is respected, otherwise there's a risk of introducing a deadlock.

[cg82616424]

Just one more thing before we start seeing to it: Did you read the LowLevelInfo.md document? It describes the already found and confirmed mutex ordering. Before you change anything in the mutex locking, make sure that the ordering there is respected, otherwise there's a risk of introducing a deadlock.

thanks for your attention, yep, I have read that document, I think I use right mutex to solve that problem

[cg82616424]

when will this pull request be merged ?

[ethouris]

The review is scheduled, but we need to include it in the plan.

[ethouris]

Ok, there are two small problems here.

1. Did you test this with SRTO_TSBPDMODE set to false? Because this code isn't going to be executed in TSBPD mode (which is default) at all - in this mode ACK action only informs TSBPD that there are potentially new packets to pickup and possibly sign off for reading when the time comes.
2. Two lines above there's the following call:

                CSync::lock_signal(m_RecvDataCond, m_RecvLock);

which is also locking m_RecvLock; locking them separately would be rather a bad idea.

Instead you should rather use the CSync object and use its signal_locked method. Follow the recvdata_cc symbol to have an example how it is done.

Actually this thing in TSBPD mode is guarded correctly - the lock is applied for a wider range and embraces setting SRT_EPOLL_IN event.

[cg82616424]

Ok, there are two small problems here.

1. Did you test this with SRTO_TSBPDMODE set to false? Because this code isn't going to be executed in TSBPD mode (which is default) at all - in this mode ACK action only informs TSBPD that there are potentially new packets to pickup and possibly sign off for reading when the time comes.
   thanks for your suggestion; yes, I tested in async mode, use file mode and message api test for about 14 hours, no deadlock occured.
2. Two lines above there's the following call:

                CSync::lock_signal(m_RecvDataCond, m_RecvLock);

which is also locking m_RecvLock; locking them separately would be rather a bad idea.
okay, I will review your soluation and update my pull request tomorrow

Instead you should rather use the CSync object and use its signal_locked method. Follow the recvdata_cc symbol to have an example how it is done.

Actually this thing in TSBPD mode is guarded correctly - the lock is applied for a wider range and embraces setting SRT_EPOLL_IN event.

[ethouris]

Ok, this is how you could do it:

{
    UniqueLock rdlock (m_RecvLock);
    CSync      rdcond (m_RecvDataCond, rdlock);

    if (m_config.bSynRecving)
    {
        // signal a waiting "recv" call if there is any data available
        rdcond.signal_locked(rdlock);
    }
    // acknowledge any waiting epolls to read
    // fix SRT_EPOLL_IN event loss but rcvbuffer still have data：
    // 1. user call receive/receivemessage(about line number:6482)
    // 2. after read/receive, if rcvbuffer is empty, will set SRT_EPOLL_IN event to false
    // 3. but if we do not do some lock work here, will cause some sync problems between threads:
    //      (1) user thread: call receive/receivemessage
    //      (2) user thread: read data
    //      (3) user thread: no data in rcvbuffer, set SRT_EPOLL_IN event to false
    //      (4) receive thread: receive data and set SRT_EPOLL_IN to true
    //      (5) user thread: set SRT_EPOLL_IN to false
    // 4. so , m_RecvLock must be used here to protect epoll event
    s_UDTUnited.m_EPoll.update_events(m_SocketID, m_sPollID, SRT_EPOLL_IN, true);
}

[cg82616424]

Ok, this is how you could do it:

{
    UniqueLock rdlock (m_RecvLock);
    CSync      rdcond (m_RecvDataCond, rdlock);

    if (m_config.bSynRecving)
    {
        // signal a waiting "recv" call if there is any data available
        rdcond.signal_locked(rdlock);
    }
    // acknowledge any waiting epolls to read
    // fix SRT_EPOLL_IN event loss but rcvbuffer still have data：
    // 1. user call receive/receivemessage(about line number:6482)
    // 2. after read/receive, if rcvbuffer is empty, will set SRT_EPOLL_IN event to false
    // 3. but if we do not do some lock work here, will cause some sync problems between threads:
    //      (1) user thread: call receive/receivemessage
    //      (2) user thread: read data
    //      (3) user thread: no data in rcvbuffer, set SRT_EPOLL_IN event to false
    //      (4) receive thread: receive data and set SRT_EPOLL_IN to true
    //      (5) user thread: set SRT_EPOLL_IN to false
    // 4. so , m_RecvLock must be used here to protect epoll event
    s_UDTUnited.m_EPoll.update_events(m_SocketID, m_sPollID, SRT_EPOLL_IN, true);
}

thanks for your solution, I have modified my pull request. plz review it again