Changeset 71080 in webkit


Ignore:
Timestamp:
Nov 1, 2010 5:09:19 PM (13 years ago)
Author:
Martin Robinson
Message:

2010-11-01 Martin Robinson <​mrobinson@igalia.com>

Reviewed by Xan Lopez.

[Soup] Random crashes in http/tests/websocket/tests/workers/worker-handshake-challenge-randomness.html
​https://bugs.webkit.org/show_bug.cgi?id=48805

Track active WebSocket handles via a sequential id. This ensures
that when a handle is reallocated into a recently used segment of
memory, it doesn't trigger a false positive in the code which ensures
the original handle is active.

No new tests. This test should stop crashing on the bots, proving the fix.

  • platform/network/soup/SocketStreamHandle.h: (WebCore::SocketStreamHandle::id): Added an m_id member and accessor to SocketStreamHandle.
  • platform/network/soup/SocketStreamHandleSoup.cpp: (WebCore::getHandleFromId): Updated to work with HashMap of handle ids to SocketStreamHandle*. (WebCore::deactivateHandle): Ditto. (WebCore::activateHandle): Ditto. (WebCore::SocketStreamHandle::SocketStreamHandle): Ditto. (WebCore::SocketStreamHandle::connected): Ditto. (WebCore::SocketStreamHandle::readBytes): Ditto. (WebCore::SocketStreamHandle::beginWaitingForSocketWritability): Ditto. (WebCore::connectedCallback): Ditto. (WebCore::readReadyCallback): Ditto. (WebCore::writeReadyCallback): Ditto.
Location:
trunk/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r71079 r71080  
     12010-11-01  Martin Robinson  <mrobinson@igalia.com>
     2
     3        Reviewed by Xan Lopez.
     4
     5        [Soup] Random crashes in http/tests/websocket/tests/workers/worker-handshake-challenge-randomness.html
     6        https://bugs.webkit.org/show_bug.cgi?id=48805
     7
     8        Track active WebSocket handles via a sequential id. This ensures
     9        that when a handle is reallocated into a recently used segment of
     10        memory, it doesn't trigger a false positive in the code which ensures
     11        the original handle is active.
     12
     13        No new tests. This test should stop crashing on the bots, proving the fix.
     14
     15        * platform/network/soup/SocketStreamHandle.h:
     16        (WebCore::SocketStreamHandle::id): Added an m_id member and accessor
     17        to SocketStreamHandle.
     18        * platform/network/soup/SocketStreamHandleSoup.cpp:
     19        (WebCore::getHandleFromId): Updated to work with HashMap of handle ids to
     20        SocketStreamHandle*.
     21        (WebCore::deactivateHandle): Ditto.
     22        (WebCore::activateHandle): Ditto.
     23        (WebCore::SocketStreamHandle::SocketStreamHandle): Ditto.
     24        (WebCore::SocketStreamHandle::connected): Ditto.
     25        (WebCore::SocketStreamHandle::readBytes): Ditto.
     26        (WebCore::SocketStreamHandle::beginWaitingForSocketWritability): Ditto.
     27        (WebCore::connectedCallback): Ditto.
     28        (WebCore::readReadyCallback): Ditto.
     29        (WebCore::writeReadyCallback): Ditto.
     30
    1312010-11-01  Kent Tamura  <tkent@chromium.org>
    232

  • trunk/WebCore/platform/network/soup/SocketStreamHandle.h

    r66986 r71080  
    5252        void readBytes(signed long, GError*);
    5353        void writeReady();
     54        void* id() { return m_id; }
    5455
    5556    protected:
    … …  
    6364        PlatformRefPtr<GSource> m_writeReadySource;
    6465        char* m_readBuffer;
     66        void* m_id;
    6567
    6668        SocketStreamHandle(const KURL&, SocketStreamHandleClient*);

  • trunk/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp

    r66986 r71080  
    4949
    5050// These functions immediately call the similarly named SocketStreamHandle methods.
    51 static void connectedCallback(GSocketClient*, GAsyncResult*, SocketStreamHandle*);
    52 static void readReadyCallback(GInputStream*, GAsyncResult*, SocketStreamHandle*);
    53 static gboolean writeReadyCallback(GSocket*, GIOCondition, SocketStreamHandle*);
     51static void connectedCallback(GSocketClient*, GAsyncResult*, void*);
     52static void readReadyCallback(GInputStream*, GAsyncResult*, void*);
     53static gboolean writeReadyCallback(GSocket*, GIOCondition, void*);
    5454
    5555// Having a list of active handles means that we do not have to worry about WebCore
    … …  
    5757// we just ignore it in the callback. We avoid a lot of extra checks and tricky
    5858// situations this way.
    59 static Vector<SocketStreamHandle*> gActiveHandles;
    60 bool isActiveHandle(SocketStreamHandle* handle)
    61 {
    62     return gActiveHandles.find(handle) != notFound;
    63 }
    64 
    65 void deactivateHandle(SocketStreamHandle* handle)
    66 {
    67     size_t handleIndex = gActiveHandles.find(handle);
    68     if (handleIndex != notFound)
    69         gActiveHandles.remove(handleIndex);
     59static HashMap<void*, SocketStreamHandle*> gActiveHandles;
     60static SocketStreamHandle* getHandleFromId(void* id)
     61{
     62    if (!gActiveHandles.contains(id))
     63        return 0;
     64    return gActiveHandles.get(id);
     65}
     66
     67static void deactivateHandle(SocketStreamHandle* handle)
     68{
     69    gActiveHandles.remove(handle->id());
     70}
     71
     72static void* activateHandle(SocketStreamHandle* handle)
     73{
     74    static gint currentHandleId = 0;
     75    void* id = GINT_TO_POINTER(currentHandleId++);
     76    gActiveHandles.set(id, handle);
     77    return id;
    7078}
    7179
    … …  
    7987    unsigned int port = url.hasPort() ? url.port() : 80;
    8088
    81     gActiveHandles.append(this);
     89    m_id = activateHandle(this);
    8290    PlatformRefPtr<GSocketClient> socketClient = adoptPlatformRef(g_socket_client_new());
    8391    g_socket_client_connect_to_host_async(socketClient.get(), url.host().utf8().data(), port, 0,
    84         reinterpret_cast<GAsyncReadyCallback>(connectedCallback), this);
     92        reinterpret_cast<GAsyncReadyCallback>(connectedCallback), m_id);
    8593}
    8694
    … …  
    105113    m_readBuffer = new char[READ_BUFFER_SIZE];
    106114    g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
    107         reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
     115        reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
    108116
    109117    // The client can close the handle, potentially removing the last reference.
    … …  
    132140    if (m_inputStream) // The client may have closed the connection.
    133141        g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
    134             reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
     142            reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
    135143}
    136144
    … …  
    216224    m_writeReadySource = adoptPlatformRef(g_socket_create_source(
    217225        g_socket_connection_get_socket(m_socketConnection.get()), static_cast<GIOCondition>(G_IO_OUT), 0));
    218     g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), this, 0);
     226    g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), m_id, 0);
    219227    g_source_attach(m_writeReadySource.get(), 0);
    220228}
    … …  
    229237}
    230238
    231 static void connectedCallback(GSocketClient* client, GAsyncResult* result, SocketStreamHandle* handle)
     239static void connectedCallback(GSocketClient* client, GAsyncResult* result, void* id)
    232240{
    233241    // Always finish the connection, even if this SocketStreamHandle was deactivated earlier.
    … …  
    236244
    237245    // The SocketStreamHandle has been deactivated, so just close the connection, ignoring errors.
    238     if (!isActiveHandle(handle)) {
     246    SocketStreamHandle* handle = getHandleFromId(id);
     247    if (!handle) {
    239248        g_io_stream_close(G_IO_STREAM(socketConnection), 0, &error.outPtr());
    240249        return;
    … …  
    244253}
    245254
    246 static void readReadyCallback(GInputStream* stream, GAsyncResult* result, SocketStreamHandle* handle)
     255static void readReadyCallback(GInputStream* stream, GAsyncResult* result, void* id)
    247256{
    248257    // Always finish the read, even if this SocketStreamHandle was deactivated earlier.
    … …  
    250259    gssize bytesRead = g_input_stream_read_finish(stream, result, &error.outPtr());
    251260
    252     if (!isActiveHandle(handle))
    253         return;
     261    SocketStreamHandle* handle = getHandleFromId(id);
     262    if (!handle)
     263        return;
     264
    254265    handle->readBytes(bytesRead, error.get());
    255266}
    256267
    257 static gboolean writeReadyCallback(GSocket*, GIOCondition condition, SocketStreamHandle* handle)
    258 {
    259     if (!isActiveHandle(handle))
     268static gboolean writeReadyCallback(GSocket*, GIOCondition condition, void* id)
     269{
     270    SocketStreamHandle* handle = getHandleFromId(id);
     271    if (!handle)
    260272        return FALSE;
    261273
Note: See TracChangeset for help on using the changeset viewer.