1 2 Previous Next 17 Replies Latest reply: May 29, 2013 1:47 PM by jtahlborn RSS

    Memory flush with synchronization block.

    1011434
       class X {
         Object myReference = null;
         private final ReentrantLock lock = new ReentrantLock();
      
         public void updateReference() {
           lock.lock();  // block until condition holds
           try {
             myReference = createNewReference();
           } finally {
             lock.unlock()
           }
         }
      
        Object getMyReference() {
           return this.myReference;
        }
       }
      Let's assume i have 2 different threads, T1 and T2.
      T1 calls updateReference, T2 calls getMyReference while T1 is inside try-catch block.

      My questions is, when global variable myReference will be flushed to global memory ? After calling lock.unlock ? Or it can happen any time between line; myReference = createNewReference(); and lock.unlock() ?

      Edited by: user8658306 on May 28, 2013 10:30 AM
        • 1. Re: Memory flush with synchronization block.
          jtahlborn
          this pattern is broken and a caller to getMyReference() can get a partially constructed object. you must lock in getMyReference() or make myReference volatile.
          • 2. Re: Memory flush with synchronization block.
            rp0428
            >
            this pattern is broken and a caller to getMyReference() can get a partially constructed object.
            >
            Huh? Please explain what you mean by 'this pattern is broken' and how any caller could 'get a partially constructed object'.

            'myReference' will either be null or will reference a valid instance. It can't reference a 'partially constructed object'.

            This line of code
            myReference = createNewReference();
            will replace the value of 'myReference' with the result of the 'createNewReference' method, whatever that may be (null or a valid instance).

            The assignment won't occur until the method completes.
            • 3. Re: Memory flush with synchronization block.
              rp0428
              Welcome to the forum!
              >
              Let's assume i have 2 different threads, T1 and T2.
              T1 calls updateReference, T2 calls getMyReference while T1 is inside try-catch block.

              My questions is, when global variable myReference will be flushed to global memory ? After calling lock.unlock ? Or it can happen any time between line; myReference = createNewReference(); and lock.unlock() ?
              >
              Tell us what PROBLEM you are trying to solve and why you think your method will solve it.

              What does 'flushed to global memory' mean to you? It has no meaning for me.

              This line of code
              myReference = createNewReference();
              will replace the value of 'myReference' with the result of the 'createNewReference' method, whatever that may be (null or a valid instance).

              The ONLY thing your use of the 'ReentrantLock' does is just what its name suggests: prevents two, or more threads from executing that method at the same time.

              A second (or third) thread will execute 'lock.lock()' and will be blocked.

              The results of ANYTHING that you do in that method (between the 'lock()' and 'unlock()' calls is immediately visible as soon as it happens.

              Again - what is it you are trying to do?
              • 4. Re: Memory flush with synchronization block.
                jtahlborn
                rp0428 wrote:
                >
                this pattern is broken and a caller to getMyReference() can get a partially constructed object.
                >
                Huh? Please explain what you mean by 'this pattern is broken' and how any caller could 'get a partially constructed object'.

                'myReference' will either be null or will reference a valid instance. It can't reference a 'partially constructed object'.

                This line of code
                myReference = createNewReference();
                will replace the value of 'myReference' with the result of the 'createNewReference' method, whatever that may be (null or a valid instance).

                The assignment won't occur until the method completes.
                i suggest you read up on multi-threading and the java memory model before continuing to comment on this thread (read up on visibility and the "happens before" relationship). since a call to getMyReference() does not share a "happens before" relationship with updateReference(), all kinds of bad things can (and will) happen.
                • 5. Re: Memory flush with synchronization block.
                  rp0428
                  And I suggest that, if you want to be helpful, you respond to what I ask you to do above.
                  >
                  Please explain what you mean by 'this pattern is broken' and how any caller could 'get a partially constructed object'.
                  >
                  I'm more than willing to 'read up' on most anything but merely saying 'read up on multi-threading and the java memory model' without providing a link or citation is next to useless. There isn't any way to correlate that to the comments you made that need clarification.

                  I note that you did not mention ANYTHING specific to these comments
                  >
                  'myReference' will either be null or will reference a valid instance. It can't reference a 'partially constructed object'.

                  This line of code
                  myReference = createNewReference();
                  will replace the value of 'myReference' with the result of the 'createNewReference' method, whatever that may be (null or a valid instance).

                  The assignment won't occur until the method completes.
                  >
                  The 'createNewReference()' method will create a new instance somewhere on the heap and, only AFTER that new instance has been constructed, will assign a new value to 'myReference'. The assignment will be the return value of the 'createNewReference' method and that will only occur when the method ends and that method won't end until AFTER the new instance has been constructed.

                  If you disagree with anything in that statement say so and explain what you disagree with, why you disagree and tell us what order you believe things will happen in.
                  • 6. Re: Memory flush with synchronization block.
                    1011434
                    Hi, thanks for comments, I give you more details about my concerns.


                    Precisely speaking I have following problem.

                    This code snippet is model of cache periodically updated (once per hour) by calling updateReference by some scheduler.

                    Many threads although want all the time access to this cache.

                    Let's assume that method createNewReference() takes a long time t1 (in production environment about 3 minutes), and getMyReference() is called very often by many threads.

                    Generally speaking my application takes some input, and produces output.
                    Each bunch of data is processed by new thread (accutally threads are pooled, and managed by executor service).

                    Processing data takes at most time t2(less thant 1 second) which is significantly less than t1 (refreshing cache).

                    So my qustion is, if calling twice getMyReference() during one cycle of processing data can return 2 different versions of cache ? Of course i dont want such situation to happen ever.
                    I don't want to get 2 version of cache in 1 cycle of processing.

                    If i can assume that t1 (refreshing cache) takes always much more time thant t2 (1 cycle of processing data), and line code unlock() is the exact moment when new version of cache is visible to accessing threads it of course can't happen.

                    Actually for my purposes it would be sufficient if I can assume, that method updateReference() if it makes new version of cache visible to other threads it makes visible in "safe" way, I mean that thread accessing cache any time gets correct reference of fully constructed new version of cache. So moment when new version of reference is visible to other threads for my purposes my can any time after createNewReference() returns fully constructed object.
                    • 7. Re: Memory flush with synchronization block.
                      gimbal2
                      jtahlborn wrote:
                      this pattern is broken and a caller to getMyReference() can get a partially constructed object. you must lock in getMyReference() or make myReference volatile.
                      • 8. Re: Memory flush with synchronization block.
                        r035198x
                        Read: https://www.securecoding.cert.org/confluence/display/java/VNA02-J.+Ensure+that+compound+operations+on+shared+variables+are+atomic

                        Assuming updateReference changes the referenced object's values (not obvious from your snippet), then:
                        The object you are using in your executors can be updated by another thread running updateReference. Just using volatile won't help either since this object is composite. Volatile will guarantee visibility but not atomicity.
                        • 9. Re: Memory flush with synchronization block.
                          gimbal2
                          Nice link!
                          • 10. Re: Memory flush with synchronization block.
                            1011434
                            Assuming updateReference changes the referenced object's values (not obvious from your snippet)
                            So let me clarify this.
                            createNewReference returns completly new instance (actually it is unmmodifiable Map).

                            After rethinking my problem I think that I made wrong assumptions.

                            Two consequentive calls to getMyReference() can return 2 different references if the first call is made right before updateReference() is finished, and second is made right after is has finished.
                            So even though updateReference wold be atomic operation I would have still problem (although different from original) and I have actually no idea how to solve it on server side.

                            I must impose some design on client code, like: when you access cache first time, pass it down to all methods if they need to access to the same version in the same processing cycle.

                            Do you have ideas how to solve this in server side code (redesigning cache holder and updater object) ?

                            Bounding cache version to thread (thread local pattern) I think is not an option because application uses thread pool.
                            • 11. Re: Memory flush with synchronization block.
                              r035198x
                              Pass the executor threads an immutable object. They would only use information they had when they started running which should be fine, otherwise your design doesn't really allow caching and must work with the latest data all the time, which would then mean that all executions must wait for updates to complete before they can run.
                              • 12. Re: Memory flush with synchronization block.
                                1011434
                                Thanks, seems like the only reasonable solution.

                                I would like not to force client code to pass down additional parameter (client may want to access cache on different levels of processing).
                                So I think I will be still forced to use ThreadLocal pattern, and at beggining of each worker, putting there ProcessingContext (containing all data, including caches) which thread can access any time it will be needed.
                                • 13. Re: Memory flush with synchronization block.
                                  jtahlborn
                                  rp0428 wrote:
                                  And I suggest that, if you want to be helpful, you respond to what I ask you to do above.
                                  >
                                  Please explain what you mean by 'this pattern is broken' and how any caller could 'get a partially constructed object'.
                                  >
                                  I'm more than willing to 'read up' on most anything but merely saying 'read up on multi-threading and the java memory model' without providing a link or citation is next to useless. There isn't any way to correlate that to the comments you made that need clarification.

                                  I note that you did not mention ANYTHING specific to these comments
                                  >
                                  'myReference' will either be null or will reference a valid instance. It can't reference a 'partially constructed object'.

                                  This line of code
                                  myReference = createNewReference();
                                  will replace the value of 'myReference' with the result of the 'createNewReference' method, whatever that may be (null or a valid instance).

                                  The assignment won't occur until the method completes.
                                  >
                                  The 'createNewReference()' method will create a new instance somewhere on the heap and, only AFTER that new instance has been constructed, will assign a new value to 'myReference'. The assignment will be the return value of the 'createNewReference' method and that will only occur when the method ends and that method won't end until AFTER the new instance has been constructed.

                                  If you disagree with anything in that statement say so and explain what you disagree with, why you disagree and tell us what order you believe things will happen in.
                                  i disagree with pretty much everything in your statements and i explained the underlying problem ("since a call to getMyReference() does not share a "happens before" relationship with updateReference(), all kinds of bad things can (and will) happen."). it's not my job to educate you as to why your answer is incorrect, but i gave you information which should guide you to plenty of online materials which will educate you. it's pretty clear that you don't understand the fundamentals of cross-thread, shared state management in java, so, again, i would recommend reading up on that before attempting to write any multi-threaded code in java (or answer questions about it in forums).
                                  • 14. Re: Memory flush with synchronization block.
                                    rp0428
                                    >
                                    i disagree with pretty much everything in your statements and i explained the underlying problem ("since a call to getMyReference() does not share a "happens before" relationship with updateReference(), all kinds of bad things can (and will) happen.").
                                    >
                                    That statement doesn't state what you disagree with or 'explain' anything at all for anyone.

                                    It's pretty clear that YOU don't understand the fundamentals of Java object-creation if you disagree with this
                                    >
                                    The 'createNewReference()' method will create a new instance somewhere on the heap.
                                    >
                                    Likewise if you are suggesting that a reference to that new instance will be accessible BEFORE the instance construction has been completed.
                                    >
                                    and, only AFTER that new instance has been constructed, will assign a new value to 'myReference'.
                                    >
                                    There IS NO reference to that new instance except internallly until the instance is actually created. You might be confusing OPs method call that returns a new instance with user code that is directly creating a new instance of a class by calling a constructor. Certainly in the latter case you CAN get access to an incompletely constructed instance and the Java Language spec itself warns you about that.
                                    >
                                    it's not my job to educate you as to why your answer is incorrect
                                    >
                                    But it IS your job to back up your statements with facts and not just a 'chicken little' response of 'all kinds of bad things can (and will) happen'. Really? Sounds interesting. Tell us what they are. Give us an example.
                                    >
                                    but i gave you information which should guide you to plenty of online materials which will educate you.
                                    >
                                    As I said above 'merely saying 'read up on multi-threading and the java memory model' without providing a link or citation is next to useless. There isn't any way to correlate that to the comments you made that need clarification.

                                    If you have something specific and to the point that supports your statement post it so we can ALL 'read up' on it.
                                    >
                                    it's pretty clear that you don't understand the fundamentals of cross-thread, shared state management in java
                                    >
                                    The only thing made clear by your initial reply and this ons is that you can't support the statements you made in your reply.
                                    >
                                    i would recommend reading up on that before attempting to write any multi-threaded code in java (or answer questions about it in forums).
                                    >
                                    And now, without possibly knowing what experience I might have in that area, you instead appear to be making naive and unwarranted assumptions about past Java projects that I have architected and implemented.

                                    I would suggest that is is YOU that should 'read up' and learn how to make credible statements that can be supported by fact before responding to ANY questions in the forums. When you make statements and aren't willing or able to back them up or make statements based on unknowable assumptions you pretty much destroy any credibility you might have been able to claim.

                                    My reply was intended to help OP understand and deal with the issue they posted. That is what the forums are for. Anyone is free to disagree with anything I said but just saying 'you are wrong, bad things will happen' isn't useful to anyone.

                                    Nothing in any of your replies directly addresses anything that OP ask about or directly addresses any of the statements that I made.
                                    1 2 Previous Next