dpolishuk / atinject

Automatically exported from code.google.com/p/atinject
0 stars 0 forks source link

Need to specify memory effects (if any) of @Inject #7

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The draft spec says nothing about how @Inject interacts with the Java 
Memory Model (JMM), but it should. 

For example, I think most people would make the implicit assumption that 
there is a happens-before edge (with the very specific meaning of "happens-
before" in the JMM) between (A) the initialization by injection of a field 
of an object of type Foo and (B) a subsequent read of that field in the 
constructor of an object with an injected Foo parameter. But the spec needs 
to spell it out before users can be sure that class Foo is thread-safe.

It's not easy to specify, however, and it gets trickier when you think 
about the possibility of pathological user-supplied Providers.

To my knowledge, no existing DI framework specifies this kind of thing. 
That doesn't let us off the hook.

Original issue reported on code.google.com by tpeie...@gmail.com on 17 Jun 2009 at 7:34

GoogleCodeExporter commented 8 years ago
FYI, fields are injected *after* the constructor returns. From the @Inject 
Javadoc,
  "Constructors are injected first, followed by fields, and then methods."

Would it suffice to mention that all injections are performed by the same 
thread, and that this happens before 
the injected instance is returned/supplied as a dependency?

There is a slight wrinkle for circular dependencies, where an injected 
dependency may not itself be fully injected.

Original comment by limpbizkit on 17 Jun 2009 at 8:01

GoogleCodeExporter commented 8 years ago
Phrases like "X after Y", "X first, followed by Y", and "X, and then Y" are not 
formal 
specifications of memory effects.

I don't think we want to require all injections to be performed in the same 
thread; it 
would limit the design space unnecessarily. In fact, I don't think it's 
achievable in 
practice; for example, injections in request scope almost certainly should be 
performed 
by the thread handling the request, not by some master injection thread.

Original comment by tpeie...@gmail.com on 17 Jun 2009 at 9:02

GoogleCodeExporter commented 8 years ago
"Happens before" is a formal specification of a memory effect; see JLS 17.4.5.

But I was too ambiguous. What I should have said is,
  "Would it suffice to mention that all injections *on an instance* are performed by the same thread, and that this 
happens before that instance is returned/supplied as a dependency?

Original comment by limpbizkit on 17 Jun 2009 at 9:29

GoogleCodeExporter commented 8 years ago
My point was that the current draft javadocs for JSR 330 don't use 
"happens-before" (note the hyphen, btw) or attempt 
to make any other kind of formal description of the required visibility 
properties of @Inject implementations. Here's 
an example of the kind of language I'd eventually like to see:

http://java.sun.com/javase/6/docs/api/java/util/concurrent/package-summary.html#
MemoryVisibility

I don't think the spec should require implementations to confine injection of 
an instance to a single thread, even if 
in practice that turns out to be the only way anyone ever implements it. 

More importantly, I'm not sure exactly how strong the visibility properties 
should be, so I think it's premature to 
try to nail it down with what might be an overly constrained spec.

I suspect that most people are implicitly assuming stronger guarantees than any 
existing DI frameworks actually 
provide. So the hard choice ahead, I think, is whether to codify existing 
practice (and tell users where it might 
differ from their expectations) or to specify those stronger guarantees (and 
maybe provide guidance to implementers on 
how to meet them).

Original comment by tpeie...@gmail.com on 17 Jun 2009 at 10:45

GoogleCodeExporter commented 8 years ago
To Jesse's point, we currently specify injection ordering within an instance, 
but we
don't say anything about ordering between instances. For example, my injectable
constructor could receive an instance which hasn't had its own fields injected 
yet.

I've been thinking about making some stronger guarantees here. For example, an
injector should inject all constructors for an object graph first and then go 
back
and inject fields and methods. This would enable you to use field and method
injection to break up circular dependencies, and it would provide a little more
determinism. Thoughts?

This gets a little tricky when you consider the fact that injectable 
constructors and
methods can call get() on a Provider, and the injector can't detect it 
statically.

As for Tim's concurrency concerns, I'm not sure if multi-threaded injection is
possible, at least not for a contiguous object graph. We'd need to specify some 
way
for the injector and the user code to coordinate, and I don't think we want to 
go there.

I do think we should specify that an injectable member should not let an 
injected
instance escape to another thread until the entire object graph construction has
completed. This will prevent user code from accessing objects that the injector 
is
concurrently injecting (allowing this would require that the injectable types
themselves be threadsafe or that the injector and user code coordinate somehow).

Original comment by crazybob...@gmail.com on 23 Jun 2009 at 10:43

GoogleCodeExporter commented 8 years ago
Specifying the order of injection makes sense, hopefully everyone's existing 
framework does it this way already ... :)

I concur wrt "concurrent" injection, I'm not sure it's possible either, and if 
it 
were I'm not confident we could spec it at all, or in a language that mere 
mortals 
(and I include myself firmly in that category). I cite the Java memory model 
spec
as an example of such ... :D

I also agree wrt to escaping references, but as you point out, user and 
injector 
code would have to coordinate, and that *could* be ugly ... perhaps this is
a topic for part #2 and not for the annotations part #1 of the spec?

Original comment by Larry.Ca...@gmail.com on 24 Jun 2009 at 12:27

GoogleCodeExporter commented 8 years ago
Any inter-thread memory guarantees here are dependent on qualifiers
(volatile or when possible final) on injectable fields themselves
and/or their usages.  They cannot be fully independently specified.
So I think the best that can be said here in the general case is that
the memory properties of injection are those of writing to the
injected fields.

Having said that, it is still unfortunate that a common usage and
commonly desired guarantee cannot be easily expressed.  Most
injectable fields are "effectively final"; i.e., written exactly once,
not necessarily within a constructor but before any inter-thread
access) is not easily guaranteed. There is some help for this coming,
in the proposed Java7 Fences API that allows manual intervention in
such cases. See discussion in the draft spec at
http://gee.cs.oswego.edu/dl/jsr166/dist/docs/java/util/concurrent/atomic/Fences.
html
While injection tools/processors might want to add for example
storeFences associated with fields as a quality of implementation
matter, they still won't allow a strong enough guarantee since
publication safety in these "manual" constructions rely on all other
usages of a field by users to also obey these "effectively final"
conventions.

-Doug

Original comment by d...@cs.oswego.edu on 24 Jun 2009 at 7:08

GoogleCodeExporter commented 8 years ago
We could just dump field and method injection. :-P

Original comment by crazybob...@gmail.com on 24 Jun 2009 at 7:11

GoogleCodeExporter commented 8 years ago
Sounds like the consensus is to leave things unspecified for now. As long as 
implementers stick to single thread per object graph injection the worst that 
can 
happen is synchronization or volatile on effectively immutable fields that 
might later 
be unnecessary.

I can live with that.

Original comment by tpeie...@gmail.com on 24 Jun 2009 at 9:21

GoogleCodeExporter commented 8 years ago
We can reopen if necessary.

Original comment by crazybob...@gmail.com on 24 Jun 2009 at 9:36