Provider<T> thread-safety

[Cow_woC]

Hi,

My Guice module statically initializes fields in my servlet. Those
fields reference DAO classes, which in turn reference EntityManager.
As a consequence,

1) The Provider<EntityManager> is shared amongst multiple threads.

2) I am forced to refer to Provider<EntityManager> instead of
EntityManager in order to ensure that each thread sees its own request-
specific EntityManager.

I dislike having to use Provider in #2 instead of referencing
EntityManager directly, but I assume there is no better way?

My second question is about the Provider interface in general. Is the
interface is meant to be used from multiple threads? What are the
actual restrictions of sharing a Provider across multiple threads? Is
it okay so long as the callers have been created by the same Injector?
Are all restrictions implementation-specific? The Javadoc doesn't say
one way or another.

More specifically, I am wondering what the restrictions are of warp-
persist's Provider<EntityManager> implementation. I assume I can share
a single provider across multiple threads even if the thread using the
Provider was not itself injected. Is that correct?

Thank you,
Gili

[Dhanji R. Prasanna]

On Fri, Jul 25, 2008 at 5:14 AM, Cow_woC <gili.t...@gmail.com> wrote:

Hi,

My Guice module statically initializes fields in my servlet. Those
fields reference DAO classes, which in turn reference EntityManager.
As a consequence,

I would strongly recommend *against* static initialization/injection unless you really have a complelling use case (e.g.: serialization). It is not really advisable for every day use, just as static fields and members aren't in general.

1) The Provider<EntityManager> is shared amongst multiple threads.

2) I am forced to refer to Provider<EntityManager> instead of
EntityManager in order to ensure that each thread sees its own request-
specific EntityManager.

I dislike having to use Provider in #2 instead of referencing
EntityManager directly, but I assume there is no better way?

EntityManager is itself not thread-safe, so this is not really desirable. One option might be to make your Daos themselves request-scoped. But this might lead to unnecessary memory overhead.

 

My second question is about the Provider interface in general. Is the
interface is meant to be used from multiple threads? What are the
actual restrictions of sharing a Provider across multiple threads? Is
it okay so long as the callers have been created by the same Injector?
Are all restrictions implementation-specific? The Javadoc doesn't say
one way or another.

It's just an interface, like any other you can have thread-safe or unsafe implementations. In general, Providers should not keep any alterable state.

More specifically, I am wondering what the restrictions are of warp-
persist's Provider<EntityManager> implementation. I assume I can share
a single provider across multiple threads even if the thread using the
Provider was not itself injected. Is that correct?

Yes you can safely share this EM provider between threads. But I'm not really sure what you mean by "thread using the provider" being injected. 

Dhanji.

[Cow_woC]

On Jul 24, 6:37 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> I would strongly recommend *against* static initialization/injection unless
> you really have a complelling use case (e.g.: serialization). It is not
> really advisable for every day use, just as static fields and members aren't
> in general.

The Guice user guide recommends using static injection for servlets.
For example, a server that contains DAO fields. This is a
serialization use-case, is it not?

> EntityManager is itself not thread-safe, so this is not really desirable.
> One option might be to make your Daos themselves request-scoped. But this
> might lead to unnecessary memory overhead.

Right, so isn't the aforementioned approach (static DAOs) better? I'm
not sure what request-scoped DAOs give you over statically-initialized
DAOs in this case.

> Yes you can safely share this EM provider between threads.

Okay, so right now I have a Provider<EntityManager> field in my DAOs.
Is there a cleaner way or is this as good as it gets?

Thank you,
Gili

[jhulford]

On Jul 24, 9:02 pm, Cow_woC <gili.tzab...@gmail.com> wrote:
> > Yes you can safely share this EM provider between threads.
>
> Okay, so right now I have a Provider<EntityManager> field in my DAOs.
> Is there a cleaner way or is this as good as it gets?

You need the warp-persist provider especially in your case because of
your static DAO's. Used in combination w/ the servlet filter, the
warp-persist provider gives you a per Thread/Request EntityManager.
If you just injected the straight EntityManager into your static DAO
you'd be sharing your EntityManager between all requests, which is a
huge no-no. I believe this is why request-scoped DAOs are slightly
nicer because you can directly inject EntityManager into them, instead
of using the provider.

[Tim Peierls]

On Jul 24, 3:14 pm, Cow_woC <gili.tzab...@gmail.com> wrote:
> My Guice module statically initializes fields in my servlet. Those
> fields reference DAO classes, which in turn reference EntityManager.
> As a consequence,
>
> 1) The Provider<EntityManager> is shared amongst multiple threads.

That's OK if EntityManager is bound in request scope (or thread
scope). From later responses, it looks like this is not true in your
case.

> 2) I am forced to refer to Provider<EntityManager> instead of
> EntityManager in order to ensure that each thread sees its own request-
> specific EntityManager.
>
> I dislike having to use Provider in #2 instead of referencing
> EntityManager directly, but I assume there is no better way?

There is. You can wrap a Provider<T> with a dynamic proxy that makes
it look like a T. Each method call on the wrapper T turns into a call
to get() on the Provider<T> to obtain the target T to which the method
call is forwarded. If you want to make a series of calls on the
wrapper T but ensure that they are all forwarded to the *same* target
T, you can use a helper class that knows how to "pin" down the proxy
for the duration of a Runnable.

In my implementation of this, ProviderProxy, you can write things
like:

Object[] toBePersisted = ...;
Provider<EntityManager> provider = ...;

EntityManager mgr = ProviderProxy.newInstance(EntityManager.class,
provider);

Query q = mgr.createQuery(qstr); // ==>
provider.get().createQuery(qstr);

// Only one provider.get() call used for all mgr calls in
Runnable:
ProviderProxy.with(mgr).atomically(new Runnable() {
public void run() {
for (Object object : toBePersisted) {
mgr.persist(object);
}
}
});

I've been meaning to blog about this technique for a while -- the
implementation of atomically() is a bit tricky -- but I keep getting
pulled away by, uh, work. If you're interested in the details, let me
know.

--tim

[Cow_woC]

If I wanted to use request-scoped DAOs, would I do the following?

1) Replace static DAO fields with non-static Provider<DAO> fields
2) Replace Provider<EntityManager> with EntityManager

and while I'm at it, I might as well move this up to the servlet/
filter level right? So instead I'd use request-scoped servlet/filters
with direct references to request-scoped DAOs that hold direct
references to EntityManager. Is that the cleanest then? :)

Gili

[Cow_woC]

Maybe I misunderstood your code, but I believe Provider.get() looks
cleaner than jumping through all these hoops. It isn't clear from your
code what happens once at initialization time and what happens inside
the find(), save() methods of the DAO.

Gili

[Tim Peierls]

If you're in a setting where Bar must be injected with a Provider<Foo>
because of potential scope incompatibilities between Foo and Bar,
ProviderProxy gives you a way to work directly with a Foo.

interface Foo {
void quux();
}
class Bar {
private final Foo foo;
@Inject Bar(Provider<Foo> fooProvider) {
this.foo = ProviderProxy.newInstance(Foo.class, fooProvider);
}
public void someMethod() {
foo.quux(); // internally becomes fooProvider.get().quux()
}
}

If you have the additional constraint of having to refer to the same
underlying target Foo over the course of several method calls, you
either need to stick with Provider<Foo> or use something like
with(...).atomically(...), which is indeed a complication.

But if you don't have that constraint, it's pretty straightforward to
use ProviderProxy.

--tim

[Sam Berlin]

I'd love to see something like ProviderProxy available as a stock
Guice thing. For instance,

class Bar {
private final Foo;
@Inject Bar(@ProviderProxy Foo foo) { // Proxies Foo by a Provider internally
this.foo = foo;
}

public void someMethod() {
foo.quux(); // internally calls fooProvider.get().quux()
}
}

Sam

[Cow_woC]

[...reposting since this failed the first time...]

The more I see this, the more I realize I don't like it.

Using Foo instead of Provider<Foo> violates the principal of least
surprise, especially given the fact that the annotation is in the
constructor instead of beside the field being proxied. If you were
looking at someone else's code and saw this you wouldn't quickly pick
up on the fact that any sort of look-up is taking place every time you
use Foo and this is dangerous.

Gili

On Jul 25, 1:32 pm, "Sam Berlin" <sber...@gmail.com> wrote:
> I'd love to see something like ProviderProxy available as a stock
> Guice thing.  For instance,
>
> class Bar {
>  private final Foo;
>  @Inject Bar(@ProviderProxy Foo foo) { // Proxies Foo by a Provider internally
>      this.foo = foo;
>   }
>
>  public void someMethod() {
>     foo.quux(); // internally calls fooProvider.get().quux()
>   }
>
> }
>
> Sam
>

[Kevin Bourrillion]

On Fri, Jul 25, 2008 at 10:53 AM, Cow_woC <gili.t...@gmail.com> wrote:

The more I see this, the more I realize I don't like it.

Using Foo instead of Provider<Foo> violates the principal of least
surprise, especially given the fact that the annotation is in the
constructor instead of beside the field being proxied. If you were
looking at someone else's code and saw this you wouldn't quickly pick
up on the fact that any sort of look-up is taking place every time you
use Foo and this is dangerous.

Well said.  This is exactly why we rejected this idea back before Guice was released.

[Dhanji R. Prasanna]

On Fri, Jul 25, 2008 at 11:40 PM, Tim Peierls <tpei...@gmail.com> wrote:

On Jul 24, 3:14 pm, Cow_woC <gili.tzab...@gmail.com> wrote:
> My Guice module statically initializes fields in my servlet. Those
> fields reference DAO classes, which in turn reference EntityManager.
> As a consequence,
>
> 1) The Provider<EntityManager> is shared amongst multiple threads.

That's OK if EntityManager is bound in request scope (or thread
scope). From later responses, it looks like this is not true in your
case.

> 2) I am forced to refer to Provider<EntityManager> instead of
> EntityManager in order to ensure that each thread sees its own request-
> specific EntityManager.
>
> I dislike having to use Provider in #2 instead of referencing
> EntityManager directly, but I assume there is no better way?

There is. You can wrap a Provider<T> with a dynamic proxy that makes
it look like a T. Each method call on the wrapper T turns into a call
to get() on the Provider<T> to obtain the target T to which the method
call is forwarded.

I refrained from suggesting this because it is a little abusive of the well-documented EntityManager interface. It is well documented as *not* threadsafe (all JPA vendors I know of follow this), and that one should be opened per unit of work. Anyone coming across the code would be pretty confused about what's going on. 

That, and using scoped-proxies always felt a bit unclean to me. (Spring allows you to do this btw). I wonder if there is a dispatch overhead too?

Providers aren't that bad, Tim =)

Dhanji.

[Dhanji R. Prasanna]

On Fri, Jul 25, 2008 at 11:02 AM, Cow_woC <gili.t...@gmail.com> wrote:

On Jul 24, 6:37 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> I would strongly recommend *against* static initialization/injection unless
> you really have a complelling use case (e.g.: serialization). It is not
> really advisable for every day use, just as static fields and members aren't
> in general.

The Guice user guide recommends using static injection for servlets.
For example, a server that contains DAO fields. This is a
serialization use-case, is it not?

It does? That's scary if it does! I could not find it in this version:

http://google-guice.googlecode.com/files/Guice%201.0%20User's%20Guide.pdf

The only two points I found about static injection were for deserialization in replicated session environments and for storing the injector for some "evil" [sic] use case.

Dhanji.

[Cow_woC]

On Jul 25, 8:09 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> > The Guice user guide recommends using static injection for servlets.
> > For example, a server that contains DAO fields. This is a
> > serialization use-case, is it not?
>

> It does? That's scary if it does! I could not find it in this version:http://google-guice.googlecode.com/files/Guice%201.0%20User's%20Guide...

You're right. I didn't read it in the user guide, I read it here:
http://groups.google.com/group/google-guice/msg/8523f1de69ca1329

> The only two points I found about static injection were for deserialization
> in replicated session environments and for storing the injector for some
> "evil" [sic] use case.

I still don't understand how servlets are not a serialization use-
case... HttpServlet implements Serializable, and as far as I
understand it is no different from a replicated session environment.
Did I miss something?

[Sam Berlin]

On Fri, Jul 25, 2008 at 2:51 PM, Kevin Bourrillion <kevi...@gmail.com> wrote:
>
> Well said. This is exactly why we rejected this idea back before Guice was
> released.

True, this is a very good point. I stepped back a bit and thought why
the pattern is compelling, and it seems the signifant use-case is when
you want to inject something, but do not want it to be constructed
immediately. That is, it enables easy lazy construction. With that
in mind it becomes clearer if the semantics & wording change. The
annotation would be @Lazy, as in:

@Inject Bar(@Lazy Foo foo) { ... }

This would instruct Guice to inject a Foo, but not actually construct
the underlying Foo until the first method is called. Internally it
would use something much like a Provider, the difference being
subsequent calls to get() return the same object. It's very much like
a non-eager Singleton within one injection scope.

Lazily constructing non-singletons today requires helper code and can
be bulky. Tim provided a nice package for simplifying it, but it
still requires the injecting a Provider of the object, and it's not
extremely clear what's going on.

Sam

[Kamil Demecki]

On Jul 25, 3:02 am, Cow_woC <gili.tzab...@gmail.com> wrote:
> Okay, so right now I have a Provider<EntityManager> field in my DAOs.
> Is there a cleaner way or is this as good as it gets?

I think this way is enough clean. EntityManager is *not* threadsafe
and Dao is. Provider interface is thin layer between these two
concerns. Every time you need enitity manager you get it from Provider.

[Tim Peierls]

If everyone's finished piling on... :-)

I was answering this question:

> 2) I am forced to refer to Provider<EntityManager> instead of
> EntityManager in order to ensure that each thread sees its own request-
> specific EntityManager.
>
> I dislike having to use Provider in #2 instead of referencing
> EntityManager directly, but I assume there is no better way?

My answer was that there is a better way if you seriously want to work
with EntityManager rather than Provider<EntityManager>. I think there
are valid reasons to want to do so in some cases (e.g., passing
injected objects to code outside of your control), but it requires
careful consideration. Providers *aren't* so bad, and it would be
wrong to use ProviderProxy simply to eliminate them out of spite or
laziness, but it would also be wrong to reject an occasionally useful
technique just because it can be misapplied.

--tim

On Jul 25, 6:57 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote: