Commit 5d5350cd authored by mrobinson@webkit.org's avatar mrobinson@webkit.org

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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71080 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ba5dfe51
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.
2010-11-01 Kent Tamura <tkent@chromium.org>
Unreviewed. Run sort-Xcode-project-file.
......@@ -51,6 +51,7 @@ namespace WebCore {
void connected(GSocketConnection*, GError*);
void readBytes(signed long, GError*);
void writeReady();
void* id() { return m_id; }
protected:
virtual int platformSend(const char* data, int length);
......@@ -62,6 +63,7 @@ namespace WebCore {
PlatformRefPtr<GOutputStream> m_outputStream;
PlatformRefPtr<GSource> m_writeReadySource;
char* m_readBuffer;
void* m_id;
SocketStreamHandle(const KURL&, SocketStreamHandleClient*);
......
......@@ -48,25 +48,33 @@
namespace WebCore {
// These functions immediately call the similarly named SocketStreamHandle methods.
static void connectedCallback(GSocketClient*, GAsyncResult*, SocketStreamHandle*);
static void readReadyCallback(GInputStream*, GAsyncResult*, SocketStreamHandle*);
static gboolean writeReadyCallback(GSocket*, GIOCondition, SocketStreamHandle*);
static void connectedCallback(GSocketClient*, GAsyncResult*, void*);
static void readReadyCallback(GInputStream*, GAsyncResult*, void*);
static gboolean writeReadyCallback(GSocket*, GIOCondition, void*);
// Having a list of active handles means that we do not have to worry about WebCore
// reference counting in GLib callbacks. Once the handle is off the active handles list
// we just ignore it in the callback. We avoid a lot of extra checks and tricky
// situations this way.
static Vector<SocketStreamHandle*> gActiveHandles;
bool isActiveHandle(SocketStreamHandle* handle)
static HashMap<void*, SocketStreamHandle*> gActiveHandles;
static SocketStreamHandle* getHandleFromId(void* id)
{
return gActiveHandles.find(handle) != notFound;
if (!gActiveHandles.contains(id))
return 0;
return gActiveHandles.get(id);
}
void deactivateHandle(SocketStreamHandle* handle)
static void deactivateHandle(SocketStreamHandle* handle)
{
size_t handleIndex = gActiveHandles.find(handle);
if (handleIndex != notFound)
gActiveHandles.remove(handleIndex);
gActiveHandles.remove(handle->id());
}
static void* activateHandle(SocketStreamHandle* handle)
{
static gint currentHandleId = 0;
void* id = GINT_TO_POINTER(currentHandleId++);
gActiveHandles.set(id, handle);
return id;
}
SocketStreamHandle::SocketStreamHandle(const KURL& url, SocketStreamHandleClient* client)
......@@ -78,10 +86,10 @@ SocketStreamHandle::SocketStreamHandle(const KURL& url, SocketStreamHandleClient
return;
unsigned int port = url.hasPort() ? url.port() : 80;
gActiveHandles.append(this);
m_id = activateHandle(this);
PlatformRefPtr<GSocketClient> socketClient = adoptPlatformRef(g_socket_client_new());
g_socket_client_connect_to_host_async(socketClient.get(), url.host().utf8().data(), port, 0,
reinterpret_cast<GAsyncReadyCallback>(connectedCallback), this);
reinterpret_cast<GAsyncReadyCallback>(connectedCallback), m_id);
}
SocketStreamHandle::~SocketStreamHandle()
......@@ -104,7 +112,7 @@ void SocketStreamHandle::connected(GSocketConnection* socketConnection, GError*
m_readBuffer = new char[READ_BUFFER_SIZE];
g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
// The client can close the handle, potentially removing the last reference.
RefPtr<SocketStreamHandle> protect(this);
......@@ -131,7 +139,7 @@ void SocketStreamHandle::readBytes(signed long bytesRead, GError* error)
m_client->didReceiveData(this, m_readBuffer, bytesRead);
if (m_inputStream) // The client may have closed the connection.
g_input_stream_read_async(m_inputStream.get(), m_readBuffer, READ_BUFFER_SIZE, G_PRIORITY_DEFAULT, 0,
reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), this);
reinterpret_cast<GAsyncReadyCallback>(readReadyCallback), m_id);
}
void SocketStreamHandle::writeReady()
......@@ -215,7 +223,7 @@ void SocketStreamHandle::beginWaitingForSocketWritability()
m_writeReadySource = adoptPlatformRef(g_socket_create_source(
g_socket_connection_get_socket(m_socketConnection.get()), static_cast<GIOCondition>(G_IO_OUT), 0));
g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), this, 0);
g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), m_id, 0);
g_source_attach(m_writeReadySource.get(), 0);
}
......@@ -228,14 +236,15 @@ void SocketStreamHandle::stopWaitingForSocketWritability()
m_writeReadySource = 0;
}
static void connectedCallback(GSocketClient* client, GAsyncResult* result, SocketStreamHandle* handle)
static void connectedCallback(GSocketClient* client, GAsyncResult* result, void* id)
{
// Always finish the connection, even if this SocketStreamHandle was deactivated earlier.
GOwnPtr<GError> error;
GSocketConnection* socketConnection = g_socket_client_connect_to_host_finish(client, result, &error.outPtr());
// The SocketStreamHandle has been deactivated, so just close the connection, ignoring errors.
if (!isActiveHandle(handle)) {
SocketStreamHandle* handle = getHandleFromId(id);
if (!handle) {
g_io_stream_close(G_IO_STREAM(socketConnection), 0, &error.outPtr());
return;
}
......@@ -243,20 +252,23 @@ static void connectedCallback(GSocketClient* client, GAsyncResult* result, Socke
handle->connected(socketConnection, error.get());
}
static void readReadyCallback(GInputStream* stream, GAsyncResult* result, SocketStreamHandle* handle)
static void readReadyCallback(GInputStream* stream, GAsyncResult* result, void* id)
{
// Always finish the read, even if this SocketStreamHandle was deactivated earlier.
GOwnPtr<GError> error;
gssize bytesRead = g_input_stream_read_finish(stream, result, &error.outPtr());
if (!isActiveHandle(handle))
SocketStreamHandle* handle = getHandleFromId(id);
if (!handle)
return;
handle->readBytes(bytesRead, error.get());
}
static gboolean writeReadyCallback(GSocket*, GIOCondition condition, SocketStreamHandle* handle)
static gboolean writeReadyCallback(GSocket*, GIOCondition condition, void* id)
{
if (!isActiveHandle(handle))
SocketStreamHandle* handle = getHandleFromId(id);
if (!handle)
return FALSE;
// G_IO_HUP and G_IO_ERR are are always active. See:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment