Skip to content
This repository has been archived by the owner on Jun 22, 2023. It is now read-only.

Add a way to bootstrap GeometryServiceProvider.Instance *only* if it's still uninitialized #41

Closed
airbreather opened this issue Apr 17, 2018 · 2 comments
Assignees

Comments

@airbreather
Copy link
Member

airbreather commented Apr 17, 2018

This is directly related to NetTopologySuite/NetTopologySuite#227, where I pointed this out in a comment.

When a downstream assembly implements GeoAPI, I expect that 99.99% of the time, that assembly has its own type that implements IGeometryServices and is expected to be the "go-to" implementation for that assembly. So when that assembly is used for the first time, it would make sense for that assembly to tell GeoAPI "hey, in case you're still looking for something to use for GeometryServiceProvider.Instance, here's one for you!", without overriding any explicit preferences that the application has already set.

In "fat" .NET target frameworks (for backwards compat reasons), when GeometryServiceProvider.Instance hasn't been explicitly initialized, the getter does its own search of the entire AppDomain looking for something that fits.

This has a few flaws:

  1. Not all .NET Standard target frameworks have what we need out-of-the-box to make this work.
  2. The search might pick up an implementation of IGeometryServiceProvider that wasn't intended to be used as a "go-to" implementation (perhaps the first thing it sees could be a "fake" that stubs out all but the specific things that a specific usage might need).
  3. There's no way for an intermediate downstream assembly to tell GeoAPI about an instance that it should use only if someone else hasn't already set their own value explicitly, not even a clean way to check if the value has already been set explicitly.
  4. The reflection-based walking is probably kinda slow.

I propose that we add something to GeometryServiceProvider that looks something like this:

public static bool SetInstanceIfUninitialized(IGeometryServices instance)
{
    return Interlocked.CompareExchange(ref _instance, instance, null) == null;
}

The return value signals true if Instance will now start using the new value, false otherwise. The return value probably won't be used, but it's basically free and maybe it could be used for logging purposes.

This directly resolves bullet point 3 from the above list.

It also allows NetTopologySuite/NetTopologySuite#227 to directly resolve bullet points 2 and 4, and indirectly resolve bullet point 1, in what's probably an extremely common case of "GeoAPI, implemented by NetTopologySuite".

There's not much of a downside to doing this in GeoAPI (downsides would be enumerated on a concrete implementation, such as the fix for NetTopologySuite/NetTopologySuite#227).

This should wait for a conclusion to the #40 / #39 story, because that directly affects this.

@YohDeadfall
Copy link
Contributor

I propose that we add something to GeometryServiceProvider that looks something like this:

Then you need an initialized instance first. So it would be better to have something like that:

public static bool Initialize(Func<IGeometryServices> factory)
{
    if (s_instance == null)
        return false;

    lock (s_lock)
    {
        // construct an instance only once even if multiple threads run initialization
        return Interlocked.CompareExchange(ref s_instance, factory(); null) == null; 
    }
}

@airbreather
Copy link
Member Author

You're right: it would have to look more like that if we settled on #39. My snippet was just to get the idea on the table (it should be correct if we went with #40, though).

airbreather added a commit that referenced this issue Apr 29, 2018
Add a method to set GeometryServiceProvider.Instance if nobody else has yet.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants