Uploaded image for project: 'Spatial Information Systems'
  1. Spatial Information Systems
  2. SIS-156

@ThreadSafe and @Immutable annotation usages are misleading



    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 0.3
    • Fix Version/s: 0.4
    • Component/s: Metadata, Referencing, Utilities
    • Labels:


      Many SIS classes having @ThreadSafe or @Immutable annotation are only tainted: the annotation applies only in a shallow sense. For example ImmutableIdentifier is truly immutable only if the Citation given to the constructor is also immutable - this citation is not cloned, since doing so would be potentially very costly (metadata tree can be depth). Furthermore, we can not force subclasses to honour the immutability contract.

      The @ThreadSafe and @Immutable annotations are strongly inspired from the Java concurrency in practice book, which uses a stricter interpretation of immutability: an immutable class can reference only immutable objects (or primitive types), or clone them. A search on internet suggests that most other libraries having such annotations interpret them in the stricter sense.

      The majority of SIS classes do not conform to the strict interpretation of immutability. This is because many objects are aggregations of other objects for which the type is an interface. We can not impose immutability or thread-safety constraints on implementations of interfaces. Consequently for the majority of SIS classes, the thread-safety behaviour is more complicated than a binary "thread-safe / not thread safe" or "immutable / mutable" binary choice.

      The proposal is to simply drop the @ThreadSafe and @Immutable annotations from SIS, and replace them by a paragraph in class javadoc.

      Javadoc examples

      A typical sentence is "The same Foo instance can be safely used by many threads without synchronization on the part of the caller. Subclasses should make sure that any overridden methods remain safe to call from multiple threads". However the following lists some examples of documentation which are difficult to catch with a simple @ThreadSafe annotation:

      • This class is thread-safe if and only if the Set given to the constructor is thread-safe.
      • This class is immutable, and thus inherently thread-safe, if the converter given to the constructor is also immutable.
      • This base class is safe for multi-threads usage. Subclasses registered in META-INF/services/ shall be thread-safe too.
      • Instances of DefaultInternationalString are thread-safe. While those instances are not strictly immutable, SIS typically references them as if they were immutable because of their add-only behaviour.
      • This class is immutable and thus inherently thread-safe if the Citation and InternationalString arguments given to the constructor are also immutable. It is caller's responsibility to ensure that those conditions hold, for example by invoking DefaultCitation.freeze() before passing the arguments to the constructor. Subclasses shall make sure that any overridden methods remain safe to call from multiple threads and do not change any public ImmutableIdentifier state.
      • This base class is immutable if the Citation, ReferenceIdentifier, GenericName and InternationalString instances given to the constructor are also immutable. Most SIS subclasses and related classes are immutable under similar conditions. This means that unless otherwise noted in the javadoc, IdentifiedObject instances created using only SIS factories and static constants can be shared by many objects and passed between threads without synchronization.
      • This class and the Latitude / Longitude subclasses are immutable, and thus inherently thread-safe. Other subclasses may or may not be immutable, at implementation choice (see java.lang.Number for an example of a similar in purpose class having mutable subclasses).


          Issue Links



              • Assignee:
                desruisseaux Martin Desruisseaux
                desruisseaux Martin Desruisseaux
              • Votes:
                0 Vote for this issue
                1 Start watching this issue


                • Created: