thread safety and AS Service implementations...

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

thread safety and AS Service implementations...

Scott Marlow
I would like to get agreement on whether we still need to protect
Service instance variables.

 From reading the javadoc in the org.jboss.msc.service.Service class,
mutable instance variables (in Service implementations) need their own
thread safety protection.

I had a mutable object reference in class PersistenceUnitService that is
set in the Service start method and cleared in stop().  Someone reported
a NPE caused by the same mutable object reference, so I think we need to
guard against this pattern elsewhere most likely (forum post about NPE
http://community.jboss.org/message/621316).

https://github.com/scottmarlow/jboss-as/commit/c0f05311680cb936fb1abcb7391d2054e16144c5 
changes the variable to be volatile.  The down side is that the object
reference won't be cached.

I don't know for sure, that the above code change will fix the NPE, just
seems likely to me.

Scott
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: thread safety and AS Service implementations...

David Lloyd-2
On 08/16/2011 06:53 PM, Scott Marlow wrote:
> I would like to get agreement on whether we still need to protect
> Service instance variables.
>
>   From reading the javadoc in the org.jboss.msc.service.Service class,
> mutable instance variables (in Service implementations) need their own
> thread safety protection.

Yes, they absolutely do... to be more specific:

1. A service's start() method may be called from any service thread.
2. A service's stop() method may be called from any service thread (in
particular, it may be different from the one which called start()).
3. Accessors may be called from any thread at all.
4. A service's getValue() not only may be called from any thread (not
just any service thread), but its value must be valid after start() is
called and before stop() is called.

> I had a mutable object reference in class PersistenceUnitService that is
> set in the Service start method and cleared in stop().  Someone reported
> a NPE caused by the same mutable object reference, so I think we need to
> guard against this pattern elsewhere most likely (forum post about NPE
> http://community.jboss.org/message/621316).

Most of our services do in fact guard against this.

> https://github.com/scottmarlow/jboss-as/commit/c0f05311680cb936fb1abcb7391d2054e16144c5
> changes the variable to be volatile.

This looks quite correct to me.

> The down side is that the object reference won't be cached.

It can in fact be cached even if it's volatile, as far as I know;
volatile just affects publication semantics.  On Intel, reading a
volatile field is basically the same as reading a non-volatile field.

> I don't know for sure, that the above code change will fix the NPE, just
> seems likely to me.

It couldn't hurt.
--
- DML
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev