Opened 15 years ago

Closed 15 years ago

#817 closed defect (fixed)

Improve Dispose thread safety for managed .NET API

Reported by: waltweltonlair Owned by: waltweltonlair
Priority: high Milestone: 2.2
Component: Web API Version: 2.0.2
Severity: minor Keywords:
Cc: External ID: 1186908

Description

The managed .NET API in MapGuide is essentially a wrapper around unmanaged objects. Some of these unmanaged objects inherit from MgDisposable instead of MgGuardDisposable, and this can result in thread-unsafe behavior during .NET garbage collection. A simple example illustrates this:

// get a managed coordinate system object from somewhere
MgCoordinateSystem mgCoordSys = ...;

// loop
for (int i=0; i<1000; ++i)
{
    // get its catalog
    MgCoordinateSystemCatalog mgCatalog = mgCoordSys.GetCatalog();

    // do something with the catalog...
}

The MgCoordinateSystem object is a wrapper around a separate unmanaged coordinate system object, and the latter stores an unmanaged catalog object. Inside the loop, each MgCoordinateSystemCatalog object that gets created is a wrapper around this same unmanaged catalog object. The ref count of the unmanaged catalog object is incremented by one for each MgCoordinateSystemCatalog we create.

In this example each MgCoordinateSystemCatalog goes out of scope at the end of the loop's block, and therefore becomes a candidate for garbage collection. At some point the garbage collector will kick in and collect unused objects, and the ref count of the unmanaged catalog object is decremented by one for each MgCoordinateSystemCatalog that's collected.

By default garbage collection happens in a separate thread, and so it's possible that garbage collection can take place at the same time as the loop is being executed. So we can have one thread (the main thread) executing the loop and incrementing the ref count of the unmanaged catalog object, and another thread (the GC thread) decrementing the ref count of the same object. Since the unmanaged catalog object inherits from MgDisposable, the calls to update the ref count are not thread-safe, and therefore the example code above can lead to data corruption and/or a crash.

The bottom line is that all unmanaged objects wrapped by managed objects need to inherit from MgGuardDisposable and not MgDisposable.

Here's the list of managed objects whose unmanaged counterparts need to change:

Web\src\DotNetApi\MgAgfReaderWriter.cs(12):public class MgAgfReaderWriter : MgDisposable {
Web\src\DotNetApi\MgByteSink.cs(12):public class MgByteSink : MgDisposable {
Web\src\DotNetApi\MgByteSource.cs(12):public class MgByteSource : MgDisposable {

Web\src\DotNetApi\MgCoordinateSystemCatalog.cs(12):public class MgCoordinateSystemCatalog : MgDisposable {
Web\src\DotNetApi\MgCoordinateSystemDictionaryUtility.cs(12):public class MgCoordinateSystemDictionaryUtility : MgDisposable {
Web\src\DotNetApi\MgCoordinateSystemEnum.cs(12):public class MgCoordinateSystemEnum : MgDisposable {
Web\src\DotNetApi\MgCoordinateSystemEnumInteger32.cs(12):public class MgCoordinateSystemEnumInteger32 : MgDisposable {
Web\src\DotNetApi\MgCoordinateSystemFactory.cs(12):public class MgCoordinateSystemFactory : MgDisposable {
Web\src\DotNetApi\MgCoordinateSystemFormatConverter.cs(12):public class MgCoordinateSystemFormatConverter : MgDisposable {

Web\src\DotNetApi\MgCoordinateCollection.cs(12):public class MgCoordinateCollection : MgDisposable,  System.Collections.Generic.IList<MgCoordinate> {
Web\src\DotNetApi\MgCoordinateIterator.cs(12):public class MgCoordinateIterator : MgDisposable {
Web\src\DotNetApi\MgCurvePolygonCollection.cs(12):public class MgCurvePolygonCollection : MgDisposable,  System.Collections.Generic.IList<MgCurvePolygon> {
Web\src\DotNetApi\MgCurveRingCollection.cs(12):public class MgCurveRingCollection : MgDisposable,  System.Collections.Generic.IList<MgCurveRing> {
Web\src\DotNetApi\MgCurveSegmentCollection.cs(12):public class MgCurveSegmentCollection : MgDisposable,  System.Collections.Generic.IList<MgCurveSegment> {
Web\src\DotNetApi\MgCurveStringCollection.cs(12):public class MgCurveStringCollection : MgDisposable,  System.Collections.Generic.IList<MgCurveString> {
Web\src\DotNetApi\MgGeometryCollection.cs(12):public class MgGeometryCollection : MgDisposable,  System.Collections.Generic.IList<MgGeometry> {
Web\src\DotNetApi\MgGeometryFactory.cs(12):public class MgGeometryFactory : MgDisposable {
Web\src\DotNetApi\MgLinearRingCollection.cs(12):public class MgLinearRingCollection : MgDisposable,  System.Collections.Generic.IList<MgLinearRing> {
Web\src\DotNetApi\MgLineStringCollection.cs(12):public class MgLineStringCollection : MgDisposable,  System.Collections.Generic.IList<MgLineString> {
Web\src\DotNetApi\MgPointCollection.cs(12):public class MgPointCollection : MgDisposable,  System.Collections.Generic.IList<MgPoint> {
Web\src\DotNetApi\MgPolygonCollection.cs(12):public class MgPolygonCollection : MgDisposable,  System.Collections.Generic.IList<MgPolygon> {

Web\src\DotNetApi\MgHttpHeader.cs(12):public class MgHttpHeader : MgDisposable {
Web\src\DotNetApi\MgHttpPrimitiveValue.cs(12):public class MgHttpPrimitiveValue : MgDisposable {
Web\src\DotNetApi\MgHttpRequest.cs(12):public class MgHttpRequest : MgDisposable {
Web\src\DotNetApi\MgHttpRequestMetadata.cs(12):public class MgHttpRequestMetadata : MgDisposable {
Web\src\DotNetApi\MgHttpRequestParam.cs(12):public class MgHttpRequestParam : MgDisposable {
Web\src\DotNetApi\MgHttpResponse.cs(12):public class MgHttpResponse : MgDisposable {
Web\src\DotNetApi\MgHttpResult.cs(12):public class MgHttpResult : MgDisposable {

Web\src\DotNetApi\MgLayerCollection.cs(12):public class MgLayerCollection : MgDisposable,  System.Collections.Generic.IList<MgLayerBase> {
Web\src\DotNetApi\MgLayerGroupCollection.cs(12):public class MgLayerGroupCollection : MgDisposable,  System.Collections.Generic.IList<MgLayerGroup> {
Web\src\DotNetApi\MgMapCollection.cs(12):public class MgMapCollection : MgDisposable,  System.Collections.Generic.IList<MgMapBase> {
Web\src\DotNetApi\MgMeasure.cs(12):public class MgMeasure : MgDisposable {
Web\src\DotNetApi\MgReadOnlyLayerCollection.cs(12):public class MgReadOnlyLayerCollection : MgDisposable,  System.Collections.Generic.IList<MgLayerBase> {

Web\src\DotNetApi\MgServerAdmin.cs(12):public class MgServerAdmin : MgDisposable {
Web\src\DotNetApi\MgSite.cs(12):public class MgSite : MgDisposable {
Web\src\DotNetApi\MgSiteConnection.cs(12):public class MgSiteConnection : MgDisposable {
Web\src\DotNetApi\MgTransform.cs(12):public class MgTransform : MgDisposable {

Web\src\DotNetApi\MgWebCommandCollection.cs(12):public class MgWebCommandCollection : MgDisposable {
Web\src\DotNetApi\MgWebLayout.cs(12):public class MgWebLayout : MgDisposable {
Web\src\DotNetApi\MgWebUiPane.cs(12):public class MgWebUiPane : MgDisposable {
Web\src\DotNetApi\MgWebWidget.cs(12):public class MgWebWidget : MgDisposable {
Web\src\DotNetApi\MgWebWidgetCollection.cs(12):public class MgWebWidgetCollection : MgDisposable {
Web\src\DotNetApi\MgWktReaderWriter.cs(12):public class MgWktReaderWriter : MgDisposable {

Change History (3)

comment:1 by waltweltonlair, 15 years ago

External ID: 1186908

comment:2 by waltweltonlair, 15 years ago

Owner: set to waltweltonlair
Status: newassigned

comment:3 by waltweltonlair, 15 years ago

Resolution: fixed
Status: assignedclosed

Fixed in the trunk stream with submission https://trac.osgeo.org/mapguide/changeset/3546.

Note: See TracTickets for help on using tickets.