⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 286961 in webkit


Ignore:
Timestamp:
Dec 13, 2021, 11:52:03 AM (5 years ago)
Author:
youenn@apple.com
Message:

REGRESSION (r286841): [ iOS ] Many webrtc tests flaky failing on iOS
​https://bugs.webkit.org/show_bug.cgi?id=234181
<rdar://problem/86343642>

Reviewed by Eric Carlson.

Use network connection state change callback to know when connection fails or is cancelled.
Introduce ConnectionStateTracker to know when to stop reading new UDP packets.
ConnectionStateTracker will be stopped when state is changed to failed or cancelled as well as when the whole connection is closed.
Renaming m_nwConnections to m_connections as m_nwConnections name was already used.
Reduce error logging to only new error codes or when connectino enters unrecoverable failure (hence changing to failed state).

Covered by existing tests.

  • NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.h:
  • NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.mm:
Location:
trunk/Source/WebKit
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r286958 r286961  
     12021-12-13  Youenn Fablet  <youenn@apple.com>
     2
     3        REGRESSION (r286841): [ iOS ] Many webrtc tests flaky failing on iOS
     4        https://bugs.webkit.org/show_bug.cgi?id=234181
     5        <rdar://problem/86343642>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Use network connection state change callback to know when connection fails or is cancelled.
     10        Introduce ConnectionStateTracker to know when to stop reading new UDP packets.
     11        ConnectionStateTracker will be stopped when state is changed to failed or cancelled as well as when the whole connection is closed.
     12        Renaming m_nwConnections to m_connections as m_nwConnections name was already used.
     13        Reduce error logging to only new error codes or when connectino enters unrecoverable failure (hence changing to failed state).
     14
     15        Covered by existing tests.
     16
     17        * NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.h:
     18        * NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.mm:
     19
    1202021-12-13  Elliott Williams  <emw@apple.com>
    221

  • trunk/Source/WebKit/NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.h

    r280556 r286961  
    7878    NetworkRTCProvider& m_rtcProvider;
    7979    WebCore::LibWebRTCSocketIdentifier m_identifier;
    80     Ref<NetworkRTCUDPSocketCocoaConnections> m_nwConnections;
     80    Ref<NetworkRTCUDPSocketCocoaConnections> m_connections;
    8181};
    8282

  • trunk/Source/WebKit/NetworkProcess/webrtc/NetworkRTCUDPSocketCocoa.mm

    r286841 r286961  
    5959    void setListeningPort(int);
    6060
     61    class ConnectionStateTracker : public ThreadSafeRefCounted<ConnectionStateTracker> {
     62    public:
     63        static Ref<ConnectionStateTracker> create() { return adoptRef(*new ConnectionStateTracker()); }
     64        void markAsStopped() { m_isStopped = true; }
     65        bool isStopped() const { return m_isStopped; }
     66
     67    private:
     68        bool m_isStopped { false };
     69    };
     70
    6171private:
    6272    NetworkRTCUDPSocketCocoaConnections(WebCore::LibWebRTCSocketIdentifier, NetworkRTCProvider&, const rtc::SocketAddress&, Ref<IPC::Connection>&&, String&& attributedBundleIdentifier, bool isFirstParty, bool isRelayDisabled, const WebCore::RegistrableDomain&);
    6373
    64     RetainPtr<nw_connection_t> createNWConnection(const rtc::SocketAddress&);
    65     void setupNWConnection(nw_connection_t, const rtc::SocketAddress&);
     74    std::pair<RetainPtr<nw_connection_t>, Ref<ConnectionStateTracker>> createNWConnection(const rtc::SocketAddress&);
     75    void setupNWConnection(nw_connection_t, ConnectionStateTracker&, const rtc::SocketAddress&);
    6676    void configureParameters(nw_parameters_t, nw_ip_version_t);
    6777
    … …  
    8090    Lock m_nwConnectionsLock;
    8191    bool m_isClosed WTF_GUARDED_BY_LOCK(m_nwConnectionsLock) { false };
    82     HashMap<rtc::SocketAddress, RetainPtr<nw_connection_t>> m_nwConnections WTF_GUARDED_BY_LOCK(m_nwConnectionsLock);
     92    HashMap<rtc::SocketAddress, std::pair<RetainPtr<nw_connection_t>, RefPtr<ConnectionStateTracker>>> m_nwConnections WTF_GUARDED_BY_LOCK(m_nwConnectionsLock);
    8393};
    8494
    … …  
    101111    : m_rtcProvider(rtcProvider)
    102112    , m_identifier(identifier)
    103     , m_nwConnections(NetworkRTCUDPSocketCocoaConnections::create(identifier, rtcProvider, address, WTFMove(connection), WTFMove(attributedBundleIdentifier), isFirstParty, isRelayDisabled, domain))
     113    , m_connections(NetworkRTCUDPSocketCocoaConnections::create(identifier, rtcProvider, address, WTFMove(connection), WTFMove(attributedBundleIdentifier), isFirstParty, isRelayDisabled, domain))
    104114{
    105115}
    … …  
    111121void NetworkRTCUDPSocketCocoa::close()
    112122{
    113     m_nwConnections->close();
     123    m_connections->close();
    114124    m_rtcProvider.takeSocket(m_identifier);
    115125}
    … …  
    117127void NetworkRTCUDPSocketCocoa::setListeningPort(int port)
    118128{
    119     m_nwConnections->setListeningPort(port);
     129    m_connections->setListeningPort(port);
    120130}
    121131
    122132void NetworkRTCUDPSocketCocoa::setOption(int option, int value)
    123133{
    124     m_nwConnections->setOption(option, value);
     134    m_connections->setOption(option, value);
    125135}
    126136
    127137void NetworkRTCUDPSocketCocoa::sendTo(const uint8_t* data, size_t size, const rtc::SocketAddress& address, const rtc::PacketOptions& options)
    128138{
    129     m_nwConnections->sendTo(data, size, address, options);
     139    m_connections->sendTo(data, size, address, options);
    130140}
    131141
    … …  
    226236    }).get());
    227237
    228     nw_listener_set_new_connection_handler(m_nwListener.get(), makeBlockPtr([protectedThis = Ref { *this }](nw_connection_t connection) {
     238    nw_listener_set_new_connection_handler(m_nwListener.get(), makeBlockPtr([protectedThis = Ref { *this }](nw_connection_t nwConnection) {
    229239        Locker locker { protectedThis->m_nwConnectionsLock };
    230240        if (protectedThis->m_isClosed)
    231241            return;
    232242
    233         auto remoteAddress = socketAddressFromIncomingConnection(connection);
     243        auto remoteAddress = socketAddressFromIncomingConnection(nwConnection);
    234244        ASSERT(remoteAddress != HashTraits<rtc::SocketAddress>::emptyValue() && !HashTraits<rtc::SocketAddress>::isDeletedValue(remoteAddress));
    235245
    236         protectedThis->m_nwConnections.set(remoteAddress, connection);
    237         protectedThis->setupNWConnection(connection, remoteAddress);
     246        auto connectionStateTracker = ConnectionStateTracker::create();
     247        protectedThis->setupNWConnection(nwConnection, connectionStateTracker.get(), remoteAddress);
     248
     249        protectedThis->m_nwConnections.set(remoteAddress, std::make_pair(nwConnection, WTFMove(connectionStateTracker)));
    238250    }).get());
    239251
    … …  
    264276    m_isClosed = true;
    265277
     278    for (auto& nwConnection : m_nwConnections.values()) {
     279        nwConnection.second->markAsStopped();
     280        nw_connection_cancel(nwConnection.first.get());
     281    }
     282    m_nwConnections.clear();
     283
    266284    nw_listener_cancel(m_nwListener.get());
    267285    m_nwListener = nullptr;
    268 
    269     for (auto& nwConnection : m_nwConnections.values())
    270         nw_connection_cancel(nwConnection.get());
    271     m_nwConnections.clear();
    272286}
    273287
    … …  
    277291}
    278292
    279 static inline void processUDPData(RetainPtr<nw_connection_t>&& nwConnection, Function<void(const uint8_t*, size_t)>&& processData)
     293static inline void processUDPData(RetainPtr<nw_connection_t>&& nwConnection, Ref<NetworkRTCUDPSocketCocoaConnections::ConnectionStateTracker> connectionStateTracker, int errorCode, Function<void(const uint8_t*, size_t)>&& processData)
    280294{
    281295    auto nwConnectionReference = nwConnection.get();
    282     nw_connection_receive(nwConnectionReference, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([nwConnection = WTFMove(nwConnection), processData = WTFMove(processData)](dispatch_data_t content, nw_content_context_t context, bool isComplete, nw_error_t error) mutable {
     296    nw_connection_receive(nwConnectionReference, 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([nwConnection = WTFMove(nwConnection), processData = WTFMove(processData), errorCode, connectionStateTracker = WTFMove(connectionStateTracker)](dispatch_data_t content, nw_content_context_t, bool, nw_error_t error) mutable {
    283297        if (content) {
    284298            dispatch_data_apply(content, makeBlockPtr([&](dispatch_data_t, size_t, const void* data, size_t size) {
    … …  
    287301            }).get());
    288302        }
    289         if (isComplete && context && nw_content_context_get_is_final(context))
     303        if (connectionStateTracker->isStopped())
    290304            return;
    291305
    292         RELEASE_LOG_ERROR_IF(!!error, WebRTC, "NetworkRTCUDPSocketCocoaConnections failed processing UDP data with error %d", nw_error_get_error_code(error));
    293         processUDPData(WTFMove(nwConnection), WTFMove(processData));
    294     }).get());
    295 }
    296 
    297 RetainPtr<nw_connection_t> NetworkRTCUDPSocketCocoaConnections::createNWConnection(const rtc::SocketAddress& remoteAddress)
     306        if (error && errorCode != nw_error_get_error_code(error)) {
     307            errorCode = nw_error_get_error_code(error);
     308            RELEASE_LOG_ERROR(WebRTC, "NetworkRTCUDPSocketCocoaConnections failed processing UDP data with error %d", errorCode);
     309        }
     310        processUDPData(WTFMove(nwConnection), WTFMove(connectionStateTracker), errorCode, WTFMove(processData));
     311    }).get());
     312}
     313
     314std::pair<RetainPtr<nw_connection_t>, Ref<NetworkRTCUDPSocketCocoaConnections::ConnectionStateTracker>> NetworkRTCUDPSocketCocoaConnections::createNWConnection(const rtc::SocketAddress& remoteAddress)
    298315{
    299316    auto parameters = adoptNS(nw_parameters_create_secure_udp(NW_PARAMETERS_DISABLE_PROTOCOL, NW_PARAMETERS_DEFAULT_CONFIGURATION));
    … …  
    322339    auto nwConnection = adoptNS(nw_connection_create(host.get(), parameters.get()));
    323340
    324     setupNWConnection(nwConnection.get(), remoteAddress);
    325     return nwConnection;
    326 }
    327 
    328 void NetworkRTCUDPSocketCocoaConnections::setupNWConnection(nw_connection_t nwConnection, const rtc::SocketAddress& remoteAddress)
     341    auto connectionStateTracker = ConnectionStateTracker::create();
     342
     343    setupNWConnection(nwConnection.get(), connectionStateTracker.get(), remoteAddress);
     344    return std::make_pair(WTFMove(nwConnection), WTFMove(connectionStateTracker));
     345}
     346
     347void NetworkRTCUDPSocketCocoaConnections::setupNWConnection(nw_connection_t nwConnection, ConnectionStateTracker& connectionStateTracker, const rtc::SocketAddress& remoteAddress)
    329348{
    330349    nw_connection_set_queue(nwConnection, udpSocketQueue());
    331350
    332     processUDPData(nwConnection, [identifier = m_identifier, connection = m_connection.copyRef(), ip = remoteAddress.ipaddr(), port = remoteAddress.port()](auto* message, auto size) mutable {
     351    nw_connection_set_state_changed_handler(nwConnection, makeBlockPtr([connectionStateTracker = Ref  { connectionStateTracker }](nw_connection_state_t state, _Nullable nw_error_t error) {
     352        RELEASE_LOG_ERROR_IF(state == nw_connection_state_failed, WebRTC, "NetworkRTCUDPSocketCocoaConnections connection failed with error %d", error ? nw_error_get_error_code(error) : 0);
     353        if (state == nw_connection_state_failed || state == nw_connection_state_cancelled)
     354            connectionStateTracker->markAsStopped();
     355    }).get());
     356
     357    processUDPData(nwConnection, Ref  { connectionStateTracker }, 0, [identifier = m_identifier, connection = m_connection.copyRef(), ip = remoteAddress.ipaddr(), port = remoteAddress.port()](auto* message, auto size) mutable {
    333358        IPC::DataReference data(message, size);
    334359        connection->send(Messages::LibWebRTCNetwork::SignalReadPacket { identifier, data, RTCNetwork::IPAddress(ip), port, rtc::TimeMillis() * 1000 }, 0);
    … …  
    350375        nwConnection = m_nwConnections.ensure(remoteAddress, [this, &remoteAddress] {
    351376            return createNWConnection(remoteAddress);
    352         }).iterator->value.get();
     377        }).iterator->value.first.get();
    353378    }
    354379
Note: See TracChangeset for help on using the changeset viewer.