Re: Feedback requested: Impending changes to RunListener

Hi all,

ok, so in that case the standard javax annotations cannot be used.

Other options are jcip and proprietary one.

I vote for jcip since the license allows us to use a dependency.

To me the name "ThreadSafe" sounds more preferable to "Concurrent", since the implemntator must take care of more than concurrent calls of listener's methods, but also atomicity, visibility of vars, constructor and treatment of critical section(s) inside of the listener.

Any thoughts?

Cheers,
Tibor

--- In [email protected], Kevin Cooney  wrote:
>
> On Tue, Feb 12, 2013 at 12:46 AM, sschroev  wrote:
> 
> > **
> >
> >
> > At the risk of repeating myself: my only objection is with the package in
> > which said annotation resides: net.jcip.annotations instead of
> > javax.annotation.concurrent.
> >
> The JSR-305 ThreadSafe annotation appears to have
> @Retention(RetentionPolicy.CLASS):
> 
> https://code.google.com/p/jsr-305/source/browse/trunk/ri/src/main/java/javax/annotation/concurrent/ThreadSafe.java
> 
> Do you see something different in your version?
> 
> Having RetentionPolicy.CLASS means it can only be read via bytecode-level
> post-processing, not via reflection:
> 
> http://stackoverflow.com/questions/3107970/annotations-retention-policy
> 
> 
> > But that argument can also be made in reverse. Hence my suggestion to
> > inspect the annotation's simple name only.
> >
> That would only work if the retention policy for your ThreadSafe annotation
> was "RUNTIME". So if we went with this approach, we would risk someone
> making a change that they believe will allow their runner to be called
> concurrently but would still result in the calls to their runner
> being synchronized.
> 
> -- Kevin
> 
> That also makes the licencing discussion mood, unless one wishes to depend
> > on jcip-annotation to provide a "default" or "preferred" version of the
> > annotation. That's fine of course. :) Though then do please mark the
> > dependency : otherwise I have to start explicitly excluding it (I derive my
> > annotations from one of the FindBugs modules mentioned in an earlier email;
> > no need to duplicate classes on the classpath).
> >
> > Cheers,
> > Stephan
> >
> >
> > --- In [email protected], "tibord47" wrote:
> > >
> > > Hi all junit guys,
> > >
> > > Let me turn back to the original problem.
> > >
> > > We knew that it's a good idea to let the users to annotated a custom
> > listener by @ThreadSafe if it's really thread-safe.
> > >
> > > In every other case the given listener is wrapped in synchronized
> > listener which is package private class.
> > >
> > > I am convinced that we should not make synchronized listener public
> > since this is our internal patch.
> > >
> > > The user may have whatever thread-safe listener depending on
> > implementation and still not having it synchronized. Thus s/he can
> > annotated the clas by @ThreadSafe.
> > > We are synchronizing the listener's methods internally in the patch only
> > because the user's implementation is unknown -listener in not annotated yet.
> > >
> > > Back to jcip-annotations license. Such library is under Creative Commons
> > Attribution License (http://creativecommons.org/licenses/by/2.5).
> > >
> > > If we use Maven dependency, we are free to share, distribute and
> > transmit.
> > > So there is no legal issue for the JUnit.
> > >
> > > Any objections to this lincense?
> > >
> > > Thx, Tibor
> > >
> > >
> > >
> > > --- In [email protected], "sschroev" wrote:
> > > >
> > > > I understand your concerns w.r.t. the intended semantics of
> > third-party @ThreadSafe annotations. For that reason I could certainly get
> > behind a differently-named annotation such as @Concurrent. Adding one more
> > annotation is cheap, after all ;)
> > > >
> > > > Cheers,
> > > > Stephan
> > > >
> > > > --- In [email protected], Kevin Cooney wrote:
> > > > >
> > > > > On Wed, Feb 6, 2013 at 10:06 AM, sschroev wrote:
> > > > >
> > > > > > **
> > > > > >
> > > > > >
> > > > > > I like the idea, a lot!
> > > > > >
> > > > > > But would like to ask that only the *simple name* of the
> > annotation is
> > > > > > considered, without regard for the package in which it resides.
> > > > > >
> > > > > Interesting idea.
> > > > >
> > > > > It looks like Guice uses this strategy for @Nullable (
> > > > > https://code.google.com/p/google-guice/wiki/UseNullable)
> > > > >
> > > > > That would require us to iterate through all of the class's
> > annotations, vs
> > > > > a simple getAnnotation() call, but listeners are not added and
> > removed very
> > > > > often.
> > > > >
> > > > > > This way one can:
> > > > > >
> > > > > > 1. Use an alternative third-party annotation, such as
> > > > > > javax.annotation.concurrent.ThreadSafe from libraries like
> > > > > > com.google.code.findbugs:jsr305:1.3.9 and
> > > > > > com.google.code.findbugs:annotations:2.0.1.
> > > > > > 2. Define a private annotation, without adding _any_ additional
> > dependency.
> > > > > >
> > > > > Alternatively, we could add a JUnit-specific annotation (possibly
> > named
> > > > > "Concurrent") and you could annotate your class with both
> > annotations (ob
> > > > > meme: http://www.quickmeme.com/meme/3svo5h/)
> > > > >
> > > > > One reservation I have with using @ThreadSafe is that usually the
> > > > > jcip-annotations documents behavior (sure, it is used by findbugs to
> > find
> > > > > potential bugs, but that's validation based on behavior documented in
> > > > > annotations). Here we are proposing using @ThreadSafe to modify
> > behavior.
> > > > > Someone might not expect by simply adding an annotation to a class
> > they are
> > > > > changing the behavior of code that uses instances of the class.
> > > > >
> > > > > >
> > > > > > I use javax.annotation.concurrent.ThreadSafe quite a lot in my own
> > code,
> > > > > > so (1) is quite important from my perspective. There are
> > precedents for
> > > > > > this approach: IIRC, Eclipse does something like this for
> > variations on
> > > > > > @Nullable and @Nonnull, whereas (again, IIRC) Findbugs handles two
> > > > > > instances of @SuppressWarnings.
> > > > > >
> > > > > > Cheers,
> > > > > > Stephan
> > > > > >
> > > > > >
> > > > > > --- In [email protected], Kevin Cooney wrote:
> > > > > > >
> > > > > > > Hello, everyone
> > > > > > >
> > > > > > > Short version: we are thinking of adding an optional dependency
> > from
> > > > > > JUnit
> > > > > > > to jcip-annotations and are soliciting feedback.
> > > > > > >
> > > > > > > A few people have noticed that when you use ParallelComputer,
> > you got
> > > > > > poor
> > > > > > > performance because calls to registered RunListeners are
> > synchronized. We
> > > > > > > would like to fix this in the next release of JUnit.
> > > > > > >
> > > > > > > In order to prevent affecting existing users, we are proposing
> > having an
> > > > > > > annotation that can be added to RunListener implementations that
> > can
> > > > > > safely
> > > > > > > be called concurrently from multiple threads. If a RunListener
> > passed to
> > > > > > a
> > > > > > > RunNotifier is not annotated with this annotation, JUnit will
> > wrap it
> > > > > > with
> > > > > > > a decorator that does synchronization.
> > > > > > >
> > > > > > >
> > > > > > > We would appreciate feedback on this new annotation.
> > > > > > >
> > > > > > > We have two options:
> > > > > > >
> > > > > > > a. Create our own annotation for this
> > > > > > > b. Use ThreadSafe (
> > > > > > >
> > > > > >
> > http://grepcode.com/file/repo1.maven.org/maven2/net.jcip/jcip-annotations/1.0/net/jcip/annotations/ThreadSafe.java
> > > > > > > )
> > > > > > >
> > > > > > > The advantages to option b are:
> > > > > > >
> > > > > > > 1. It's a well-known annotation
> > > > > > > 2. Users that use JUnit can add this annotation to their
> > thread-safe
> > > > > > > RunListener implementations today and get the benefits "for
> > free" later.
> > > > > > >
> > > > > > > Note we can do this without requiring that all users of JUnit
> > have the
> > > > > > > jcip-annotations jar in their classpath as of JDK 1.5.0_06 (see
> > > > > > >
> > > > > >
> > http://stackoverflow.com/questions/3567413/why-doesnt-a-missing-annotation-cause-a-classnotfoundexception-at-runtime
> > > > > > ).
> > > > > > > If jcip-annotations is not on the classpath then all RunListener
> > > > > > > implementations will be treated as not thread-safe.
> > > > > > >
> > > > > > > As a specific example, the Maven Surefire plugin would use this
> > > > > > dependency
> > > > > > > and include jcip-annotations.
> > > > > > >
> > > > > > > Currently, JUnit only depends on one third-party library
> > (Hamcrest). This
> > > > > > > has caused us some pain in the past when the Hamcrest code
> > changed in
> > > > > > ways
> > > > > > > that broke existing users of JUnit. We believe this will not be
> > an issue
> > > > > > > with jcip-annotations because the JAR only contains annotations,
> > not any
> > > > > > > code.
> > > > > > >
> > > > > > > One possible concern would be with other open source libraries
> > that
> > > > > > provide
> > > > > > > RunListener implementations (they would need to add
> > jcip-annotations to
> > > > > > > their compile-time classpath if they have thread-safe
> > annotations, and
> > > > > > > document this to their users).
> > > > > > >
> > > > > > > Thoughts?
> > > > > > >
> > > > > > > -- Kevin
> > > > > > >
> > > > > > >
> > > > > > > [Non-text portions of this message have been removed]
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > [Non-text portions of this message have been removed]
> > > > >
> > > >
> > >
> >
> >  
> >
> 
> 
> [Non-text portions of this message have been removed]
>




------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/junit/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/junit/join
    (Yahoo! ID required)

<*> To change settings via email:
    [email protected] 
    [email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/