Changeset 173720 in webkit


Ignore:
Timestamp:
Sep 18, 2014 6:06:59 AM (10 years ago)
Author:
zandobersek@gmail.com
Message:

GMainLoopSource is exposed to race conditions
​https://bugs.webkit.org/show_bug.cgi?id=135800

Reviewed by Carlos Garcia Campos.

Source/WTF:

GMainLoopSource objects can be dispatching tasks on one thread
while having a new task scheduled on a different thread. This
can for instance occur in WebKitVideoSink, where the timeout
callback can be called on main thread while at the same time
it is being rescheduled on a different thread (created through
GStreamer).

The initial solution is to use GMutex to prevent parallel data
access from different threads. In the future I plan to look at
the possibility of creating thread-specific GMainLoopSource
objects that wouldn't require the use of GMutex.

GSource, GCancellable and std::function<> objects are now packed
into an internal Context structure. Using the C++11 move semantics
it's simple to, at the time of dispatch, move the current context
out of the GMainLoopSource object in case the dispatch causes a
rescheduling on that same object.

Also added in the Context struct is a new GCancellable. The pointer
of that object is shared with the GMainLoopSource before the Context
is moved out for the callback dispatch. This makes it safe to cancel
or even delete the GMainLoopSource during the dispatch and prevents
use-after-delete on GMainLoopSource once the dispatch is done in
the GMainLoopSource::*Callback() methods.

All the schedule*() methods and the cancelWithoutLocking() method
callers now lock the GMutex to ensure no one else is accessing the
data at that moment. Similar goes for the dispatch methods, but those
do the dispatch and possible destruction duties with the mutex unlocked.
The dispatch can cause rescheduling on the same GMainLoopSource object,
which must not be done with a locked mutex.

  • wtf/gobject/GMainLoopSource.cpp:

(WTF::GMainLoopSource::GMainLoopSource):
(WTF::GMainLoopSource::~GMainLoopSource):
(WTF::GMainLoopSource::cancel):
(WTF::GMainLoopSource::cancelWithoutLocking):
(WTF::GMainLoopSource::scheduleIdleSource):
(WTF::GMainLoopSource::schedule):
(WTF::GMainLoopSource::scheduleTimeoutSource):
(WTF::GMainLoopSource::scheduleAfterDelay):
(WTF::GMainLoopSource::voidCallback):
(WTF::GMainLoopSource::boolCallback):
(WTF::GMainLoopSource::socketCallback):
(WTF::GMainLoopSource::socketSourceCallback):
(WTF::GMainLoopSource::Context::destroySource):
(WTF::GMainLoopSource::reset): Deleted.
(WTF::GMainLoopSource::destroy): Deleted.

  • wtf/gobject/GMainLoopSource.h:

Tools:

Add unit tests for GMainLoopSource.

The tests check correct behavior of GMainLoopSource in various conditions --
from the most simple rescheduling to rescheduling during dispatch, cancelling
or destroying the GMainLoopSource during dispatch, proper destroy callback
dispatching etc.

Scheduling both void (one-time) and bool (repeatable) callbacks is tested.
State of the GMainLoopSource object (either ready, sheduled or active) is
thoroughly tested throughout the lifetime of that object.

Still missing are tests for socket callbacks, which are a bit trickier because
they rely on a GSocket object. The delete-on-destroy GMainLoopSource objects
are also not tested thoroughly, simply because it is at the moment impossible
to test that the objects are actually destroyed when the corresponding source
is finally deleted.

  • TestWebKitAPI/PlatformGTK.cmake:
  • TestWebKitAPI/Tests/WTF/gobject/GMainLoopSource.cpp: Added.

(TestWebKitAPI::GMainLoopSourceTest::GMainLoopSourceTest):
(TestWebKitAPI::GMainLoopSourceTest::~GMainLoopSourceTest):
(TestWebKitAPI::GMainLoopSourceTest::runLoop):
(TestWebKitAPI::GMainLoopSourceTest::delayedFinish):
(TestWebKitAPI::GMainLoopSourceTest::finish):
(TestWebKitAPI::GMainLoopSourceTest::source):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
1 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r173710 r173720  
     12014-09-18  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        GMainLoopSource is exposed to race conditions
     4        https://bugs.webkit.org/show_bug.cgi?id=135800
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        GMainLoopSource objects can be dispatching tasks on one thread
     9        while having a new task scheduled on a different thread. This
     10        can for instance occur in WebKitVideoSink, where the timeout
     11        callback can be called on main thread while at the same time
     12        it is being rescheduled on a different thread (created through
     13        GStreamer).
     14
     15        The initial solution is to use GMutex to prevent parallel data
     16        access from different threads. In the future I plan to look at
     17        the possibility of creating thread-specific GMainLoopSource
     18        objects that wouldn't require the use of GMutex.
     19
     20        GSource, GCancellable and std::function<> objects are now packed
     21        into an internal Context structure. Using the C++11 move semantics
     22        it's simple to, at the time of dispatch, move the current context
     23        out of the GMainLoopSource object in case the dispatch causes a
     24        rescheduling on that same object.
     25
     26        Also added in the Context struct is a new GCancellable. The pointer
     27        of that object is shared with the GMainLoopSource before the Context
     28        is moved out for the callback dispatch. This makes it safe to cancel
     29        or even delete the GMainLoopSource during the dispatch and prevents
     30        use-after-delete on GMainLoopSource once the dispatch is done in
     31        the GMainLoopSource::*Callback() methods.
     32
     33        All the schedule*() methods and the cancelWithoutLocking() method
     34        callers now lock the GMutex to ensure no one else is accessing the
     35        data at that moment. Similar goes for the dispatch methods, but those
     36        do the dispatch and possible destruction duties with the mutex unlocked.
     37        The dispatch can cause rescheduling on the same GMainLoopSource object,
     38        which must not be done with a locked mutex.
     39
     40        * wtf/gobject/GMainLoopSource.cpp:
     41        (WTF::GMainLoopSource::GMainLoopSource):
     42        (WTF::GMainLoopSource::~GMainLoopSource):
     43        (WTF::GMainLoopSource::cancel):
     44        (WTF::GMainLoopSource::cancelWithoutLocking):
     45        (WTF::GMainLoopSource::scheduleIdleSource):
     46        (WTF::GMainLoopSource::schedule):
     47        (WTF::GMainLoopSource::scheduleTimeoutSource):
     48        (WTF::GMainLoopSource::scheduleAfterDelay):
     49        (WTF::GMainLoopSource::voidCallback):
     50        (WTF::GMainLoopSource::boolCallback):
     51        (WTF::GMainLoopSource::socketCallback):
     52        (WTF::GMainLoopSource::socketSourceCallback):
     53        (WTF::GMainLoopSource::Context::destroySource):
     54        (WTF::GMainLoopSource::reset): Deleted.
     55        (WTF::GMainLoopSource::destroy): Deleted.
     56        * wtf/gobject/GMainLoopSource.h:
     57
    1582014-09-17  Daniel Bates  <dabates@apple.com>
    259

  • trunk/Source/WTF/wtf/gobject/GMainLoopSource.cpp

    r173267 r173720  
    2929
    3030#include "GMainLoopSource.h"
    31 
    3231#include <gio/gio.h>
     32#include <wtf/gobject/GMutexLocker.h>
    3333
    3434namespace WTF {
    … …  
    4343    , m_status(Ready)
    4444{
     45    g_mutex_init(&m_mutex);
    4546}
    4647
    … …  
    4950    , m_status(Ready)
    5051{
     52    g_mutex_init(&m_mutex);
    5153}
    5254
    … …  
    5456{
    5557    cancel();
     58    g_mutex_clear(&m_mutex);
    5659}
    5760
    … …  
    6871void GMainLoopSource::cancel()
    6972{
    70     if (!m_source)
     73    GMutexLocker locker(m_mutex);
     74    cancelWithoutLocking();
     75}
     76
     77void GMainLoopSource::cancelWithoutLocking()
     78{
     79    // A valid context should only be present if GMainLoopSource is in the Scheduled or Dispatching state.
     80    ASSERT(!m_context.source || m_status == Scheduled || m_status == Dispatching);
     81    // The general cancellable object should only be present if we're currently dispatching this GMainLoopSource.
     82    ASSERT(!m_cancellable || m_status == Dispatching);
     83    // Delete-on-destroy GMainLoopSource objects can only be cancelled when there's callback either scheduled
     84    // or in the middle of dispatch. At that point cancellation will have no effect.
     85    ASSERT(m_deleteOnDestroy != DeleteOnDestroy || (m_status == Ready && !m_context.source));
     86
     87    m_status = Ready;
     88
     89    // The source is perhaps being cancelled in the middle of a callback dispatch.
     90    // Cancelling this GCancellable object will convey this information to the
     91    // current execution context when the callback dispatch is finished.
     92    g_cancellable_cancel(m_cancellable.get());
     93    m_cancellable = nullptr;
     94    g_cancellable_cancel(m_context.socketCancellable.get());
     95
     96    if (!m_context.source)
    7197        return;
    7298
    73     GRefPtr<GSource> source;
    74     m_source.swap(source);
    75 
    76     if (m_cancellable)
    77         g_cancellable_cancel(m_cancellable.get());
    78     g_source_destroy(source.get());
    79     destroy();
    80 }
    81 
    82 void GMainLoopSource::reset()
    83 {
    84     m_status = Ready;
    85     m_source = nullptr;
    86     m_cancellable = nullptr;
    87     m_voidCallback = nullptr;
    88     m_boolCallback = nullptr;
    89     m_destroyCallback = nullptr;
     99    Context context = WTF::move(m_context);
     100    context.destroySource();
    90101}
    91102
    … …  
    95106    m_status = Scheduled;
    96107
    97     m_source = adoptGRef(g_idle_source_new());
    98     g_source_set_name(m_source.get(), name);
     108    g_source_set_name(m_context.source.get(), name);
    99109    if (priority != G_PRIORITY_DEFAULT_IDLE)
    100         g_source_set_priority(m_source.get(), priority);
    101     g_source_set_callback(m_source.get(), sourceFunction, this, nullptr);
    102     g_source_attach(m_source.get(), context);
     110        g_source_set_priority(m_context.source.get(), priority);
     111    g_source_set_callback(m_context.source.get(), sourceFunction, this, nullptr);
     112    g_source_attach(m_context.source.get(), context);
    103113}
    104114
    105115void GMainLoopSource::schedule(const char* name, std::function<void ()> function, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    106116{
    107     cancel();
    108     m_voidCallback = WTF::move(function);
    109     m_destroyCallback = WTF::move(destroyFunction);
     117    GMutexLocker locker(m_mutex);
     118    cancelWithoutLocking();
     119
     120    ASSERT(!m_context.source);
     121    m_context = {
     122        adoptGRef(g_idle_source_new()),
     123        adoptGRef(g_cancellable_new()),
     124        nullptr, // socketCancellable
     125        WTF::move(function),
     126        nullptr, // boolCallback
     127        nullptr, // socketCallback
     128        WTF::move(destroyFunction)
     129    };
    110130    scheduleIdleSource(name, reinterpret_cast<GSourceFunc>(voidSourceCallback), priority, context);
    111131}
    … …  
    113133void GMainLoopSource::schedule(const char* name, std::function<bool ()> function, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    114134{
    115     cancel();
    116     m_boolCallback = WTF::move(function);
    117     m_destroyCallback = WTF::move(destroyFunction);
     135    GMutexLocker locker(m_mutex);
     136    cancelWithoutLocking();
     137
     138    ASSERT(!m_context.source);
     139    m_context = {
     140        adoptGRef(g_idle_source_new()),
     141        adoptGRef(g_cancellable_new()),
     142        nullptr, // socketCancellable
     143        nullptr, // voidCallback
     144        WTF::move(function),
     145        nullptr, // socketCallback
     146        WTF::move(destroyFunction)
     147    };
    118148    scheduleIdleSource(name, reinterpret_cast<GSourceFunc>(boolSourceCallback), priority, context);
    119149}
    … …  
    121151void GMainLoopSource::schedule(const char* name, std::function<bool (GIOCondition)> function, GSocket* socket, GIOCondition condition, std::function<void ()> destroyFunction, GMainContext* context)
    122152{
    123     cancel();
     153    GMutexLocker locker(m_mutex);
     154    cancelWithoutLocking();
     155
     156    // Don't allow scheduling GIOCondition callbacks on delete-on-destroy GMainLoopSources.
     157    ASSERT(m_deleteOnDestroy == DoNotDeleteOnDestroy);
     158
     159    ASSERT(!m_context.source);
     160    GCancellable* socketCancellable = g_cancellable_new();
     161    m_context = {
     162        adoptGRef(g_socket_create_source(socket, condition, socketCancellable)),
     163        adoptGRef(g_cancellable_new()),
     164        adoptGRef(socketCancellable),
     165        nullptr, // voidCallback
     166        nullptr, // boolCallback
     167        WTF::move(function),
     168        WTF::move(destroyFunction)
     169    };
     170
    124171    ASSERT(m_status == Ready);
    125172    m_status = Scheduled;
    126 
    127     m_socketCallback = WTF::move(function);
    128     m_destroyCallback = WTF::move(destroyFunction);
    129     m_cancellable = adoptGRef(g_cancellable_new());
    130     m_source = adoptGRef(g_socket_create_source(socket, condition, m_cancellable.get()));
    131     g_source_set_name(m_source.get(), name);
    132     g_source_set_callback(m_source.get(), reinterpret_cast<GSourceFunc>(socketSourceCallback), this, nullptr);
    133     g_source_attach(m_source.get(), context);
     173    g_source_set_name(m_context.source.get(), name);
     174    g_source_set_callback(m_context.source.get(), reinterpret_cast<GSourceFunc>(socketSourceCallback), this, nullptr);
     175    g_source_attach(m_context.source.get(), context);
    134176}
    135177
    … …  
    139181    m_status = Scheduled;
    140182
    141     ASSERT(m_source);
    142     g_source_set_name(m_source.get(), name);
     183    g_source_set_name(m_context.source.get(), name);
    143184    if (priority != G_PRIORITY_DEFAULT)
    144         g_source_set_priority(m_source.get(), priority);
    145     g_source_set_callback(m_source.get(), sourceFunction, this, nullptr);
    146     g_source_attach(m_source.get(), context);
     185        g_source_set_priority(m_context.source.get(), priority);
     186    g_source_set_callback(m_context.source.get(), sourceFunction, this, nullptr);
     187    g_source_attach(m_context.source.get(), context);
    147188}
    148189
    149190void GMainLoopSource::scheduleAfterDelay(const char* name, std::function<void ()> function, std::chrono::milliseconds delay, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    150191{
    151     cancel();
    152     m_source = adoptGRef(g_timeout_source_new(delay.count()));
    153     m_voidCallback = WTF::move(function);
    154     m_destroyCallback = WTF::move(destroyFunction);
     192    GMutexLocker locker(m_mutex);
     193    cancelWithoutLocking();
     194
     195    ASSERT(!m_context.source);
     196    m_context = {
     197        adoptGRef(g_timeout_source_new(delay.count())),
     198        adoptGRef(g_cancellable_new()),
     199        nullptr, // socketCancellable
     200        WTF::move(function),
     201        nullptr, // boolCallback
     202        nullptr, // socketCallback
     203        WTF::move(destroyFunction)
     204    };
    155205    scheduleTimeoutSource(name, reinterpret_cast<GSourceFunc>(voidSourceCallback), priority, context);
    156206}
    … …  
    158208void GMainLoopSource::scheduleAfterDelay(const char* name, std::function<bool ()> function, std::chrono::milliseconds delay, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    159209{
    160     cancel();
    161     m_source = adoptGRef(g_timeout_source_new(delay.count()));
    162     m_boolCallback = WTF::move(function);
    163     m_destroyCallback = WTF::move(destroyFunction);
     210    GMutexLocker locker(m_mutex);
     211    cancelWithoutLocking();
     212
     213    ASSERT(!m_context.source);
     214    m_context = {
     215        adoptGRef(g_timeout_source_new(delay.count())),
     216        adoptGRef(g_cancellable_new()),
     217        nullptr, // socketCancellable
     218        nullptr, // voidCallback
     219        WTF::move(function),
     220        nullptr, // socketCallback
     221        WTF::move(destroyFunction)
     222    };
    164223    scheduleTimeoutSource(name, reinterpret_cast<GSourceFunc>(boolSourceCallback), priority, context);
    165224}
    … …  
    167226void GMainLoopSource::scheduleAfterDelay(const char* name, std::function<void ()> function, std::chrono::seconds delay, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    168227{
    169     cancel();
    170     m_source = adoptGRef(g_timeout_source_new_seconds(delay.count()));
    171     m_voidCallback = WTF::move(function);
    172     m_destroyCallback = WTF::move(destroyFunction);
     228    GMutexLocker locker(m_mutex);
     229    cancelWithoutLocking();
     230
     231    ASSERT(!m_context.source);
     232    m_context = {
     233        adoptGRef(g_timeout_source_new_seconds(delay.count())),
     234        adoptGRef(g_cancellable_new()),
     235        nullptr, // socketCancellable
     236        WTF::move(function),
     237        nullptr, // boolCallback
     238        nullptr, // socketCallback
     239        WTF::move(destroyFunction)
     240    };
    173241    scheduleTimeoutSource(name, reinterpret_cast<GSourceFunc>(voidSourceCallback), priority, context);
    174242}
    … …  
    176244void GMainLoopSource::scheduleAfterDelay(const char* name, std::function<bool ()> function, std::chrono::seconds delay, int priority, std::function<void ()> destroyFunction, GMainContext* context)
    177245{
    178     cancel();
    179     m_source = adoptGRef(g_timeout_source_new_seconds(delay.count()));
    180     m_boolCallback = WTF::move(function);
    181     m_destroyCallback = WTF::move(destroyFunction);
     246    GMutexLocker locker(m_mutex);
     247    cancelWithoutLocking();
     248
     249    ASSERT(!m_context.source);
     250    m_context = {
     251        adoptGRef(g_timeout_source_new_seconds(delay.count())),
     252        adoptGRef(g_cancellable_new()),
     253        nullptr, // socketCancellable
     254        nullptr, // voidCallback
     255        WTF::move(function),
     256        nullptr, // socketCallback
     257        WTF::move(destroyFunction)
     258    };
    182259    scheduleTimeoutSource(name, reinterpret_cast<GSourceFunc>(boolSourceCallback), priority, context);
    183260}
    … …  
    185262void GMainLoopSource::voidCallback()
    186263{
    187     if (!m_source)
     264    Context context;
     265
     266    {
     267        GMutexLocker locker(m_mutex);
     268        if (!m_context.source)
     269            return;
     270
     271        context = WTF::move(m_context);
     272
     273        ASSERT(context.voidCallback);
     274        ASSERT(m_status == Scheduled);
     275        m_status = Dispatching;
     276
     277        m_cancellable = context.cancellable;
     278    }
     279
     280    context.voidCallback();
     281
     282    if (g_cancellable_is_cancelled(context.cancellable.get())) {
     283        context.destroySource();
    188284        return;
    189 
    190     ASSERT(m_voidCallback);
    191     ASSERT(m_status == Scheduled);
    192     m_status = Dispatched;
    193 
    194     GSource* source = m_source.get();
    195     m_voidCallback();
    196     if (source == m_source.get())
    197         destroy();
     285    }
     286
     287    bool shouldSelfDestruct = false;
     288    {
     289        GMutexLocker locker(m_mutex);
     290        m_status = Ready;
     291        m_cancellable = nullptr;
     292        shouldSelfDestruct = m_deleteOnDestroy == DeleteOnDestroy;
     293    }
     294
     295    context.destroySource();
     296    if (shouldSelfDestruct)
     297        delete this;
    198298}
    199299
    200300bool GMainLoopSource::boolCallback()
    201301{
    202     if (!m_source)
    203         return false;
    204 
    205     ASSERT(m_boolCallback);
    206     ASSERT(m_status == Scheduled || m_status == Dispatched);
    207     m_status = Dispatched;
    208 
    209     GSource* source = m_source.get();
    210     bool retval = m_boolCallback();
    211     if (!retval && source == m_source.get())
    212         destroy();
     302    Context context;
     303
     304    {
     305        GMutexLocker locker(m_mutex);
     306        if (!m_context.source)
     307            return Stop;
     308
     309        context = WTF::move(m_context);
     310
     311        ASSERT(context.boolCallback);
     312        ASSERT(m_status == Scheduled || m_status == Dispatching);
     313        m_status = Dispatching;
     314
     315        m_cancellable = context.cancellable;
     316    }
     317
     318    bool retval = context.boolCallback();
     319
     320    if (g_cancellable_is_cancelled(context.cancellable.get())) {
     321        context.destroySource();
     322        return Stop;
     323    }
     324
     325    bool shouldSelfDestruct = false;
     326    {
     327        GMutexLocker locker(m_mutex);
     328        m_cancellable = nullptr;
     329        shouldSelfDestruct = m_deleteOnDestroy == DeleteOnDestroy;
     330
     331        // m_status should reflect whether the GMainLoopSource has been rescheduled during dispatch.
     332        ASSERT((!m_context.source && m_status == Dispatching) || m_status == Scheduled);
     333        if (retval && !m_context.source)
     334            m_context = WTF::move(context);
     335        else if (!retval)
     336            m_status = Ready;
     337    }
     338
     339    if (context.source) {
     340        context.destroySource();
     341        if (shouldSelfDestruct)
     342            delete this;
     343    }
    213344
    214345    return retval;
    … …  
    217348bool GMainLoopSource::socketCallback(GIOCondition condition)
    218349{
    219     if (!m_source)
    220         return false;
    221 
    222     ASSERT(m_socketCallback);
    223     ASSERT(m_status == Scheduled || m_status == Dispatched);
    224     m_status = Dispatched;
    225 
    226     if (g_cancellable_is_cancelled(m_cancellable.get())) {
    227         destroy();
    228         return false;
    229     }
    230 
    231     GSource* source = m_source.get();
    232     bool retval = m_socketCallback(condition);
    233     if (!retval && source == m_source.get())
    234         destroy();
     350    Context context;
     351
     352    {
     353        GMutexLocker locker(m_mutex);
     354        if (!m_context.source)
     355            return Stop;
     356
     357        context = WTF::move(m_context);
     358
     359        ASSERT(context.socketCallback);
     360        ASSERT(m_status == Scheduled || m_status == Dispatching);
     361        m_status = Dispatching;
     362
     363        m_cancellable = context.cancellable;
     364    }
     365
     366    if (g_cancellable_is_cancelled(context.socketCancellable.get())) {
     367        context.destroySource();
     368        return Stop;
     369    }
     370
     371    bool retval = context.socketCallback(condition);
     372
     373    if (g_cancellable_is_cancelled(context.cancellable.get())) {
     374        context.destroySource();
     375        return Stop;
     376    }
     377
     378    {
     379        GMutexLocker locker(m_mutex);
     380        m_cancellable = nullptr;
     381
     382        // m_status should reflect whether the GMainLoopSource has been rescheduled during dispatch.
     383        ASSERT((!m_context.source && m_status == Dispatching) || m_status == Scheduled);
     384
     385        if (retval && !m_context.source)
     386            m_context = WTF::move(context);
     387        else if (!retval)
     388            m_status = Ready;
     389    }
     390
     391    if (context.source)
     392        context.destroySource();
    235393
    236394    return retval;
    237395}
    238396
    239 void GMainLoopSource::destroy()
    240 {
    241     auto destroyCallback = WTF::move(m_destroyCallback);
    242     auto deleteOnDestroy = m_deleteOnDestroy;
    243     reset();
     397gboolean GMainLoopSource::voidSourceCallback(GMainLoopSource* source)
     398{
     399    source->voidCallback();
     400    return G_SOURCE_REMOVE;
     401}
     402
     403gboolean GMainLoopSource::boolSourceCallback(GMainLoopSource* source)
     404{
     405    return source->boolCallback() == Continue;
     406}
     407
     408gboolean GMainLoopSource::socketSourceCallback(GSocket*, GIOCondition condition, GMainLoopSource* source)
     409{
     410    return source->socketCallback(condition) == Continue;
     411}
     412
     413void GMainLoopSource::Context::destroySource()
     414{
     415    g_source_destroy(source.get());
    244416    if (destroyCallback)
    245417        destroyCallback();
    246 
    247     if (deleteOnDestroy == DoNotDeleteOnDestroy)
    248         return;
    249 
    250     delete this;
    251 }
    252 
    253 gboolean GMainLoopSource::voidSourceCallback(GMainLoopSource* source)
    254 {
    255     source->voidCallback();
    256     return G_SOURCE_REMOVE;
    257 }
    258 
    259 gboolean GMainLoopSource::boolSourceCallback(GMainLoopSource* source)
    260 {
    261     return source->boolCallback() == Continue;
    262 }
    263 
    264 gboolean GMainLoopSource::socketSourceCallback(GSocket*, GIOCondition condition, GMainLoopSource* source)
    265 {
    266     return source->socketCallback(condition) == Continue;
    267418}
    268419

  • trunk/Source/WTF/wtf/gobject/GMainLoopSource.h

    r173267 r173720  
    3333
    3434typedef struct _GSocket GSocket;
     35typedef union _GMutex GMutex;
    3536
    3637namespace WTF {
    … …  
    6465    GMainLoopSource(DeleteOnDestroyType);
    6566
    66     enum Status { Ready, Scheduled, Dispatched };
     67    enum Status { Ready, Scheduled, Dispatching };
    6768
    68     void reset();
     69    void cancelWithoutLocking();
    6970    void scheduleIdleSource(const char* name, GSourceFunc, int priority, GMainContext*);
    7071    void scheduleTimeoutSource(const char* name, GSourceFunc, int priority, GMainContext*);
    … …  
    7273    bool boolCallback();
    7374    bool socketCallback(GIOCondition);
     75
    7476    void destroy();
    7577
    … …  
    8082    DeleteOnDestroyType m_deleteOnDestroy;
    8183    Status m_status;
    82     GRefPtr<GSource> m_source;
     84    GMutex m_mutex;
    8385    GRefPtr<GCancellable> m_cancellable;
    84     std::function<void ()> m_voidCallback;
    85     std::function<bool ()> m_boolCallback;
    86     std::function<bool (GIOCondition)> m_socketCallback;
    87     std::function<void ()> m_destroyCallback;
     86
     87    struct Context {
     88        Context() = default;
     89        Context(Context&&) = default;
     90        Context& operator=(Context&&) = default;
     91
     92        void destroySource();
     93
     94        GRefPtr<GSource> source;
     95        GRefPtr<GCancellable> cancellable;
     96        GRefPtr<GCancellable> socketCancellable;
     97        std::function<void ()> voidCallback;
     98        std::function<bool ()> boolCallback;
     99        std::function<bool (GIOCondition)> socketCallback;
     100        std::function<void ()> destroyCallback;
     101    } m_context;
    88102};
    89103

  • trunk/Tools/ChangeLog

    r173711 r173720  
     12014-09-18  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        GMainLoopSource is exposed to race conditions
     4        https://bugs.webkit.org/show_bug.cgi?id=135800
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Add unit tests for GMainLoopSource.
     9
     10        The tests check correct behavior of GMainLoopSource in various conditions --
     11        from the most simple rescheduling to rescheduling during dispatch, cancelling
     12        or destroying the GMainLoopSource during dispatch, proper destroy callback
     13        dispatching etc.
     14
     15        Scheduling both void (one-time) and bool (repeatable) callbacks is tested.
     16        State of the GMainLoopSource object (either ready, sheduled or active) is
     17        thoroughly tested throughout the lifetime of that object.
     18
     19        Still missing are tests for socket callbacks, which are a bit trickier because
     20        they rely on a GSocket object. The delete-on-destroy GMainLoopSource objects
     21        are also not tested thoroughly, simply because it is at the moment impossible
     22        to test that the objects are actually destroyed when the corresponding source
     23        is finally deleted.
     24
     25        * TestWebKitAPI/PlatformGTK.cmake:
     26        * TestWebKitAPI/Tests/WTF/gobject/GMainLoopSource.cpp: Added.
     27        (TestWebKitAPI::GMainLoopSourceTest::GMainLoopSourceTest):
     28        (TestWebKitAPI::GMainLoopSourceTest::~GMainLoopSourceTest):
     29        (TestWebKitAPI::GMainLoopSourceTest::runLoop):
     30        (TestWebKitAPI::GMainLoopSourceTest::delayedFinish):
     31        (TestWebKitAPI::GMainLoopSourceTest::finish):
     32        (TestWebKitAPI::GMainLoopSourceTest::source):
     33        (TestWebKitAPI::TEST):
     34
    1352014-09-17  Ryuan Choi  <ryuan.choi@gmail.com>
    236

  • trunk/Tools/TestWebKitAPI/PlatformGTK.cmake

    r173267 r173720  
    137137
    138138list(APPEND TestWTF_SOURCES
     139    ${TESTWEBKITAPI_DIR}/Tests/WTF/gobject/GMainLoopSource.cpp
    139140    ${TESTWEBKITAPI_DIR}/Tests/WTF/gobject/GUniquePtr.cpp
    140141)
Note: See TracChangeset for help on using the changeset viewer.