Fixing GeoCoordinateWatcher's event handlers

Identifying and fixing a bug in GeoCoordinateWatcher's event handlers.


I was recently doing some work with GeoCoordinateWatcher, and noticed some strange behaviour with the way its event handlers are wired up internally.

Just for some context, GeoCoordinateWatcher implements the IGeoPositionWatcher interface, which defines the PositionChanged and StatusChanged events.

What's wrong with GeoCoordinateWatcher?

My problems started when I noticed that the events mentioned above weren't always firing as expected. Further investigation indicated that the event handlers would only work as intended when the variable was typed as GeoCoordinateWatcher, not as IGeoPositionWatcher. See example:

/* This code works: */
GeoCoordinateWatcher w = new GeoCoordinateWatcher();
w.StatusChanged += /* insert event handler here */
w.PositionChanged += /* insert event handler here */
w.Start();
/* This code does NOT work: */
IGeoPositionWatcher<GeoCoordinate> w = new GeoCoordinateWatcher();
w.StatusChanged += /* insert event handler here */
w.PositionChanged += /* insert event handler here */
w.Start();

Bizarre, right? Instances of the same class, with different behaviour based on the type of the variable we use to refer to them. Obviously there's an explicit interface implementation being made, but what exactly is going on, and why is it breaking things?

Reflector to the rescue

Inspecting the decompiled source of GeoCoordinateWatcher (you can use dotPeek, Reflector, or dotnetinside to do this) provided some further clarity.

Here is the relevant source, with the irrelevant bits cut out:

/* The PositionChanged and StatusChanged event properties are declared as normal. */

public event EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>> PositionChanged;

public event EventHandler<GeoPositionStatusChangedEventArgs> StatusChanged;

/* The IGeoPositionWatcher<GeoCoordinate>.PositionChanged and  
   IGeoPositionWatcher<GeoCoordinate>.StatusChanged event
   properties are declared as explicit interface overrides.
   Their add/remove delegates are also explicitly implemented. */

private EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>> m_positionChanged;

private EventHandler<GeoPositionStatusChangedEventArgs> m_statusChanged;

event EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>> 
    IGeoPositionWatcher<GeoCoordinate>.PositionChanged
{
    [SecuritySafeCritical]
    add
    {
        m_positionChanged =
            (EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>>)
                Delegate.Combine(m_positionChanged, value);
    }
    [SecuritySafeCritical]
    remove
    {
        m_positionChanged =
            (EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>>)
                Delegate.Remove(m_positionChanged, value);
    }
}

event EventHandler<GeoPositionStatusChangedEventArgs>
    IGeoPositionWatcher<GeoCoordinate>.StatusChanged
{
    [SecuritySafeCritical]
    add
    {
        m_statusChanged =
            (EventHandler<GeoPositionStatusChangedEventArgs>)
                Delegate.Combine(m_statusChanged, value);
    }
    [SecuritySafeCritical]
    remove
    {
        m_statusChanged =
            (EventHandler<GeoPositionStatusChangedEventArgs>)
                Delegate.Remove(m_statusChanged, value);
    }
}

/* OnPositionChanged raises GeoCoordinateWatcher.PositionChanged,
   but never IGeoPositionWatcher<GeoCoordinate>.PositionChanged */

protected void OnPositionChanged(GeoPositionChangedEventArgs<GeoCoordinate> e)
{
    var positionChanged = PositionChanged;
    if (positionChanged != null)
    {
        positionChanged(this, e);
    }
}

/* OnPositionStatusChanged raises GeoCoordinateWatcher.StatusChanged,
   but never IGeoPositionWatcher<GeoCoordinate>.StatusChanged */

protected void OnPositionStatusChanged(GeoPositionStatusChangedEventArgs e)
{
    var statusChanged = StatusChanged;
    if (statusChanged != null)
    {
        statusChanged(this, e);
    }
}

I've added some comments to the code to explain what is going on. The events are declared as expected, but then some explicit interface implementations are also declared. For whatever reason, the explicit interface implementations of the events are kept COMPLETELY SEPARATE from the base events, and the OnPositionChanged / OnPositionStatusChanged methods only raise the base events - meaning that the explicitly implemented events are NEVER raised.

How do we fix it?

Luckily, there's an easy fix. We can just extend GeoCoordinateWatcher and ensure that the events are wired up how they should be.

class FixedGeoCoordinateWatcher : GeoCoordinateWatcher, IGeoPositionWatcher<GeoCoordinate>
{
    event EventHandler<GeoPositionChangedEventArgs<GeoCoordinate>> 
        IGeoPositionWatcher<GeoCoordinate>.PositionChanged
    {
        add { base.PositionChanged += value; }
        remove { base.PositionChanged -= value; }
    }

    event EventHandler<GeoPositionStatusChangedEventArgs> 
        IGeoPositionWatcher<GeoCoordinate>.StatusChanged
    {
        add { base.StatusChanged += value; }
        remove { base.StatusChanged -= value; }
    }
}

A complete copy of this fix (including XML documentation, etc.) is available at https://gist.github.com/MatthewKing/10222964


Posted by Matthew King on 12 April 2014
Permission is granted to use all code snippets under CC BY-SA 3.0 (just like StackOverflow), or the MIT license - your choice!