Changeset 71119 in webkit


Ignore:
Timestamp:
Nov 2, 2010 8:52:36 AM (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

    r71117 r71119  
     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-02  Csaba Osztrogonác  <ossy@webkit.org>
    232

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

    r71091 r71119  
    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

    r71091 r71119  
    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    // The first id cannot be 0, because it conflicts with the HashMap emptyValue.
     75    static gint currentHandleId = 1;
     76    void* id = GINT_TO_POINTER(currentHandleId++);
     77    gActiveHandles.set(id, handle);
     78    return id;
    7079}
    7180
    … …  
    7988    unsigned int port = url.hasPort() ? url.port() : 80;
    8089
    81     gActiveHandles.append(this);
     90    m_id = activateHandle(this);
    8291    PlatformRefPtr<GSocketClient> socketClient = adoptPlatformRef(g_socket_client_new());
    8392    g_socket_client_connect_to_host_async(socketClient.get(), url.host().utf8().data(), port, 0,
    84         reinterpret_cast<GAsyncReadyCallback>(connectedCallback), this);
     93        reinterpret_cast<GAsyncReadyCallback>(connectedCallback), m_id);
    8594}
    8695
    … …  
    105114    m_readBuffer = new char[READ_BUFFER_SIZE];
    106115    g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
    107         reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
     116        reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
    108117
    109118    // The client can close the handle, potentially removing the last reference.
    … …  
    132141    if (m_inputStream) // The client may have closed the connection.
    133142        g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
    134             reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
     143            reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
    135144}
    136145
    … …  
    216225    m_writeReadySource = adoptPlatformRef(g_socket_create_source(
    217226        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);
     227    g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), m_id, 0);
    219228    g_source_attach(m_writeReadySource.get(), 0);
    220229}
    … …  
    229238}
    230239
    231 static void connectedCallback(GSocketClient* client, GAsyncResult* result, SocketStreamHandle* handle)
     240static void connectedCallback(GSocketClient* client, GAsyncResult* result, void* id)
    232241{
    233242    // Always finish the connection, even if this SocketStreamHandle was deactivated earlier.
    … …  
    236245
    237246    // The SocketStreamHandle has been deactivated, so just close the connection, ignoring errors.
    238     if (!isActiveHandle(handle)) {
     247    SocketStreamHandle* handle = getHandleFromId(id);
     248    if (!handle) {
    239249        g_io_stream_close(G_IO_STREAM(socketConnection), 0, &error.outPtr());
    240250        return;
    … …  
    244254}
    245255
    246 static void readReadyCallback(GInputStream* stream, GAsyncResult* result, SocketStreamHandle* handle)
     256static void readReadyCallback(GInputStream* stream, GAsyncResult* result, void* id)
    247257{
    248258    // Always finish the read, even if this SocketStreamHandle was deactivated earlier.
    … …  
    250260    gssize bytesRead = g_input_stream_read_finish(stream, result, &error.outPtr());
    251261
    252     if (!isActiveHandle(handle))
    253         return;
     262    SocketStreamHandle* handle = getHandleFromId(id);
     263    if (!handle)
     264        return;
     265
    254266    handle->readBytes(bytesRead, error.get());
    255267}
    256268
    257 static gboolean writeReadyCallback(GSocket*, GIOCondition condition, SocketStreamHandle* handle)
    258 {
    259     if (!isActiveHandle(handle))
     269static gboolean writeReadyCallback(GSocket*, GIOCondition condition, void* id)
     270{
     271    SocketStreamHandle* handle = getHandleFromId(id);
     272    if (!handle)
    260273        return FALSE;
    261274
Note: See TracChangeset for help on using the changeset viewer.