0

In the Proxy object (the object implementing java.lang.reflect.InvocationHandler), I am trying to set an instance variable in the proxied object.

Like the following:

public class ServiceProxy implements InvocationHandler {

    private final Object proxiedObject;

    private ServiceProxy(final Object object) {
        this.proxiedObject = object;
    }

    public static Object newInstance(final Object object) {
        return Proxy.newProxyInstance(object.getClass().getClassLoader(), object.getClass().getInterfaces(), new ServiceProxy(object));
    }


    public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {

        Object result = null;
        MyObject mo = new MyObject();

        // Is the following safe when the proxiedObject is being acceessed by multiple threads?
        final Field sessionField = this.proxiedObject.getClass().getSuperclass().getDeclaredField("mo");
        sessionField.setAccessible(true);
        sessionField.set(this.object, mo);


        result = method.invoke(this.proxiedObject, args);


        return result;
    }
}

Is this safe?

EDIT:

Actual code:

Object result = null;
Session session = HibernateUtil.getSessionFactory().getCurrentSession()

// Is the following save when the proxiedObject is being acceessed by multiple threads?
final Field sessionField = this.proxiedObject.getClass().getSuperclass().getDeclaredField("session");
sessionField.setAccessible(true);
sessionField.set(this.object, session);

result = method.invoke(this.proxiedObject, args);
return result;

Edit2: The proxied object is being called from GWT client that calls multiple methods of the same proxied object at the same time. When this happens, I got the session instance field (of proxied class) to be closed and opened in unexpected manner.

Muhammad Hewedy
  • 29,102
  • 44
  • 127
  • 219
  • what is the _actual_ problem you are having. you hinted at a "threading issue" below, what is it? – jtahlborn Jul 19 '12 at 16:44
  • i don't understand what you mean about "closed and opened in unexpected manner". secondly, if you are implying that you are using the hibernate Session from multiple threads, you should know that Hibernate Sessions are _not_ thread-safe. – jtahlborn Jul 19 '12 at 16:52
  • Thread 1 open session, thread 1 use open session .. Then thread 2 request to open the session if not already opened.. Then thread 1 close the session .. Then thread 2 try to use the session (that was closed) and hence an exception arise here.... – Muhammad Hewedy Jul 19 '12 at 16:58
  • I think using `SessionFactory` will solve the problem.. correct? – Muhammad Hewedy Jul 19 '12 at 16:59
  • yes, each thread should be using its own instance of Session (using the SessionFactory to acquire a Session instance as needed). – jtahlborn Jul 19 '12 at 17:06
  • My Issue fixed by replacing `Session` With `SessionFactory` Thanks Guyz. – Muhammad Hewedy Jul 19 '12 at 17:27

1 Answers1

2

Is the following safe when the proxiedObject is being acceessed by multiple threads?

No, unless mo is volatile. If the mo field is volatile then this will properly cross a memory barrier and the updates to the mo field will be seen by all threads.

It is important to realize that if the MyObject is not immutable, additional concurrency issues will result even if mo is volatile.


Edit:

Thanks to @jtahlborn comments on this. I've been doing some reading and I'm now pretty sure that the volatile will also protect the MyObject from being partially initialized.

Due to constructor instruction reordering possibilities, there is no guarantee that the MyObject has been fulling constructed when its referenced is shared between threads. Only final fields in a constructor are guaranteed to be properly initialized when a constructor finishes. Any other fields may or may not have been initialized and so you will need to synchronize on the object before the multiple threads can start accessing it.

However, if themo field is volatile, then the "happens before" guarantee also ensures that the MyObject has been fully initialized. The compiler is not allowed to reorder those instructions past the publishing of a volatile variable. See:

Community
  • 1
  • 1
Gray
  • 115,027
  • 24
  • 293
  • 354
  • 1
    if the `mo` field is volatile, then the MyObject will be correctly published due to volatile semantics. MyObject does not need to be immutable or have all final fields or any other crazy requirements. (obviously if `mo` is _not_ volatile then all bets are off). – jtahlborn Jul 19 '12 at 16:38
  • mm, Hence the problem in in being MyObject is immutable or not.. so Let me put my actual code willing you can help me ... please see my edit. – Muhammad Hewedy Jul 19 '12 at 16:41
  • @jtahlborn actually, I've `mo` volatile but still have the threading issues! – Muhammad Hewedy Jul 19 '12 at 16:42
  • The problem is not about publishing @jtahlborn but about instruction reordering. Any of the constructor instructions that have been completed by the time the `volatile` field is assigned _will_ be published along with the `mo`, however the JVM makes not guarantee about non-final field initialization. – Gray Jul 19 '12 at 16:44
  • 1
    the jdk 5 memory model strengthened volatile so that _nothing_ can be re-ordered across it. if this were not the case, ReentrantLock, ConcurrentHashMap and most of the rest of the new concurrent utils would be useless. volatile now has the same strength synchronized in terms of memory visibility. – jtahlborn Jul 19 '12 at 16:45
  • Can you answer jtahlborn's question @Muhammad? Can you be more specific about the "threading issues" you are having? – Gray Jul 19 '12 at 16:46
  • You may be right @jtahlborn although I've been failing in an attempt to find a specific reference to volatiles and constructors. Can you find one? – Gray Jul 19 '12 at 16:51
  • 2
    [Synchronization Order](http://docs.oracle.com/javase/specs/jls/se5.0/html/memory.html#17.4.4), bullet point 2. there's nothing specific to constructors there because nothing specific is needed. everything that happens before the volatile write (object construction) is visible after the volatile read. just like assignment in a synchronized block. – jtahlborn Jul 19 '12 at 16:59
  • Thanks @jtahlborn. I've found some more specific information about constructors and I've updated my answer. – Gray Jul 19 '12 at 17:02
  • @jtahlborn Does that not guarantee "only" that whatever the constructor is doing will be visible by the time the volatile is read, but not necessarily right after it has been written? – assylias Jul 19 '12 at 17:03
  • That's what I thought too @assylias. As I do some reading however, accessing a `volatile` in any way insures the "happens before" guarantee. The compiler is not allowed to reorder _any_ instructions past such a synchronization point. – Gray Jul 19 '12 at 17:08
  • Guyz, I don't understand anything here :) – Muhammad Hewedy Jul 19 '12 at 17:14
  • @Gray The way I read it (with w = volatile write, r = volatile read): w synchronizes with r => hb(w, r). But if the order of execution is: construction of object -> w -> read object fields -> r, I am not 100% sure that the "read object fields" is in a hb relationship with the construction. If "read object fields" were after r in execution order then I agree that there would be no problem any longer. – assylias Jul 19 '12 at 17:16
  • @Muhammad I think your problems are not with the publishing of the object. I suspect that the session is being properly initialized and properly published but there is some other race condition here. – Gray Jul 19 '12 at 17:17
  • @Muhammad - this discussion is unrelated to your issue, your problem is the incorrect usage of the Session. – jtahlborn Jul 19 '12 at 17:21
  • 1
    @assylias - you can't read the object fields in another thread without first reading the volatile reference (obviously i'm assuming that the volatile reference is the means of publishing the object). so it would be "construction of object(t1) -> w(t1) -> r(t2) -> read object fields(t2)" (using t1 and t2 to represent two separate threads). – jtahlborn Jul 19 '12 at 17:22
  • 2
    Yeah @assylias. I see no way for the order of execution to be: object construction -> w -> read object fields -> r. The only thread that would be able to read the object fields _before_ reading the `volatile` variable would be the first thread which wouldn't have an issue. – Gray Jul 19 '12 at 17:29