synchronization is not working right between two copies of the same class for member function

Discussion in 'Java' started by John T. Kerich, Nov 25, 2006.

  1. I have a global class that was created to hold the references to different
    classes and global variables. This global class is initialized by the main
    class (some variable are pass in as arguments) and all the other threads do
    a "gd = new Global()" to get their own copy of the class (this should be
    done by the threads after initialization was done by main). The idea was
    that since all the variables are saved in the class as "static" the other
    threads would be able to access any variables saved in the global class
    without parent class passing down a pointer to the global class reference.
    I have found however that is design is not work correctly because one or
    more threads are setting the same variables at that same time or
    setting/getting a variable at the same time in the global class.

    The original coder added while loop (see below) for the members that he want
    to be synchronized using a static locking flag, but in the debugger (and in
    real life) I can see both threads can see active as false and the while loop
    fails to stop one of the threads from executing the members code while the
    other is doing it!

    static boolean active = false;

    void synchronized connect() {
    while (active) {
    wait(50);
    }
    active = true;
    member code
    active = false;
    }

    To try and fix the problem I removed the while loop and adding
    synchronization to one of the member declarations, but that didn't work
    either! I can see the main and one of the child threads calling the
    initializing a socket connect member at the same time even after I added the
    synchronization to the function declaration. I assume that synchronize will
    only work for the same class reference and not between two copies on the
    same class. I really don't want to recode the application and pass the
    global class reference down to every thread/class that needs to call global
    so that synchronization will work, but it looks like that's what I am going
    to have to do to get synchronize to work.

    Any ideas on how to fix this design problem without a major rewrite?
    John T. Kerich, Nov 25, 2006
    #1
    1. Advertising

  2. Re: synchronization is not working right between two copies of thesame class for member function

    John T. Kerich wrote:
    > I have a global class that was created to hold the references to different
    > classes and global variables. This global class is initialized by the main
    > class (some variable are pass in as arguments) and all the other threads do
    > a "gd = new Global()" to get their own copy of the class (this should be
    > done by the threads after initialization was done by main). The idea was

    ....
    > static boolean active = false;
    >
    > void synchronized connect() {
    > while (active) {
    > wait(50);
    > }
    > active = true;
    > member code
    > active = false;
    > }

    ....

    A synchronized member method, such as connect(), is synchronized on
    "this". Since each thread has its own Global, each synchronizes on a
    different object.

    You need to synchronize on something for which there is one object for
    the whole class. Here are some options:

    1. Synchronize on Global.class.

    2. Make active a Boolean, and synchronize on active.

    3. Create an artificial, static, activeLock object and synchronize on it.

    Patricia
    Patricia Shanahan, Nov 25, 2006
    #2
    1. Advertising

  3. John T. Kerich

    Eric Sosman Guest

    Re: synchronization is not working right between two copies of thesame class for member function

    John T. Kerich wrote:
    > I have a global class that was created to hold the references to different
    > classes and global variables. This global class is initialized by the main
    > class (some variable are pass in as arguments) and all the other threads do
    > a "gd = new Global()" to get their own copy of the class (this should be
    > done by the threads after initialization was done by main).


    This is probably the immediate cause of your problems. Each
    thread gets its own unique instance of Global to have and to hold.
    If forty threads all do this, there will be forty distinct Global
    objects floating around in the program. Make sure you understand
    that, and then read on ...

    > The idea was
    > that since all the variables are saved in the class as "static" the other
    > threads would be able to access any variables saved in the global class
    > without parent class passing down a pointer to the global class reference.


    Here's the second point to ponder: No matter how many instances
    of Global there are, there will be one and only one of each of the
    Global class' static variables. If Global has `static short shrift;'
    as a member, there will be exactly one Global.shrift even if there
    are forty different Global instances. (There will be exactly one
    Global.shrift even if there are *zero* Global instances, assuming
    the Global class has been loaded at all.)

    Note the discrepancy: Forty Global instances, but only one
    Global.shrift. That's not necessarily a problem, but in this
    case ... Keep reading.

    > I have found however that is design is not work correctly because one or
    > more threads are setting the same variables at that same time or
    > setting/getting a variable at the same time in the global class.
    >
    > The original coder added while loop (see below) for the members that he want
    > to be synchronized using a static locking flag, but in the debugger (and in
    > real life) I can see both threads can see active as false and the while loop
    > fails to stop one of the threads from executing the members code while the
    > other is doing it!
    >
    > static boolean active = false;
    >
    > void synchronized connect() {
    > while (active) {
    > wait(50);
    > }
    > active = true;
    > member code
    > active = false;
    > }


    (I'm assuming that both `active' and `connect' are in the
    Global class.)

    Question: How many different threads can execute connect()
    simultaneously? Well, how many different Global objects are
    there that a thread might choose to synchronize on? The first
    thread synchronizes on its own Global object, acquires the object's
    lock and plunges into connect(). Meanwhile, the second thread
    synchronizes on its Global object, acquires that object's lock,
    and also plunges into connect(). And the third, and fourth, and
    although it might be something of an unlikely coincidence you'll
    find that, yes, all forty threads could be executing connect()
    simultaneously, each having locked its very own Global instance.

    Question II: What have the forty threads done to stay out
    of each other's way while mucking with the single Global.active
    variable, the one that belongs to the whole Global class rather
    than to any particular Global instance? They've done absolutely
    nothing at all, that's what. So if all forty threads enter
    connect() at the same time, many of them will execute the `while'
    and see that active is false, and all of those will proceed
    merrily to set it to true while they execute the `member code'.

    On the evidence available, it seems the "original coder" you
    mention did not understand synchronization. Instead of using one
    traffic light to control access to a busy intersection, he's given
    every car its own private light to consult.

    > To try and fix the problem I removed the while loop and adding
    > synchronization to one of the member declarations, but that didn't work
    > either!


    By now, I hope you can see why.

    > I can see the main and one of the child threads calling the
    > initializing a socket connect member at the same time even after I added the
    > synchronization to the function declaration.


    Ah, `synchronized' was yours? Perhaps I begin to get a glimmer
    of the original coder's motivation: He may have heard, somewhere,
    that "synchronizing is slow" and tried to avoid it. And he wasn't
    bright enough to realize that by doing so he created a problem that
    not even the addition of `synchronized' can cure. (But read on ...)

    > I assume that synchronize will
    > only work for the same class reference and not between two copies on the
    > same class. I really don't want to recode the application and pass the
    > global class reference down to every thread/class that needs to call global
    > so that synchronization will work, but it looks like that's what I am going
    > to have to do to get synchronize to work.


    It's not at all clear what you mean by "work." The `synchronized'
    keyword "works" as designed, but I have a suspicion that you don't
    understand the design.

    > Any ideas on how to fix this design problem without a major rewrite?


    It's easy enough the fix the immediate problem. The thing you're
    trying to control access to ("protect") with synchronization is the
    collection of static variables belonging to the Global class. You
    can only protect them if all threads agree not to touch them except
    when synchronized on the very same object. It can be any object you
    like, as long as all the threads can find it and synchronize on it;
    for reasons of convenience and convention, a good candidate would be
    the Class object for the Global class itself, Global.class.

    How to synchronize on Global.class? You could do it explicitly:

    void connect() {
    synchronized(Global.class) {
    method code
    }
    }

    .... but there's an even more convenient way:

    synchronized static void connect() {
    method code
    }

    What's the difference between this and the original (aside from the
    jettisoning of that silly `active' boolean and the useless cruft
    that accompanied it)? In this version, connect() is a static method
    rather than an instance method. So `synchronized' doesn't mean to
    acquire the lock on `this' object -- there is no `this' for a static
    method -- but to acquire the lock on the Class object. Bingo!
    Problem solved.

    ... except that your grasp of synchronization seems so tenuous
    at this point that I fear this may merely peel one layer off the
    onion. For example: Have you noticed that the `new Global()' in
    every thread is completely useless? If not, you haven't quite
    understood things yet, and I foresee further difficulties. You
    really, really need some education before trying to proceed; even
    in Java, it takes discipline to write correct threaded code. You
    are unlikely to get there just by trying things at random and hoping
    for the best; even if you "get there," you won't have a reliable way
    of knowing you've made it! The Java Tutorial has some material on
    threading that may help, and any good textbook should explain it;
    I strongly advise you to hit the books before coding yourself into
    confusion. Good luck!

    --
    Eric Sosman
    lid
    Eric Sosman, Nov 25, 2006
    #3
  4. John T. Kerich

    Guest

    I think Eric Sosman that you should re-read the post since I thought I
    made it clear that I already know that the while loop code the previous
    coder wrote did not work and I was trying to get synchronization to
    work with the existing class as coded. As for the new Global being
    "completely useless" you missed the point that the class reference was
    not passed into any of the threads which started this problem in the
    first place because the old coder did not understand that
    synchronization would not work for copies of the class (And I didn't
    either at first because all the Java books are always passing in the
    class reference to the threads or the runable classes are local to a
    controller class which has saved the class reference in a static
    variable that is then used by the local threads). And yes I have read
    3 Java books and find their chapters on synchronization and threads
    simplistic and missing good real world examples and my group doesn't
    have any Java people with lots of experience with Java, which is why I
    post this in the first pass.

    As for making synchronization work by making the member static,
    "synchronized static void", I was thinking that was the way to go over
    the weekend, so thanks to Patricia and Eric for confirming that's the
    way to fix this problem.
    , Nov 26, 2006
    #4
  5. John T. Kerich

    andyb Guest

    Re: synchronization is not working right between two copies of thesameclass for member function

    Eric Sosman wrote:
    > John T. Kerich wrote:
    >> I have a global class that was created to hold the references to
    >> different
    >> classes and global variables. This global class is initialized by the
    >> main
    >> class (some variable are pass in as arguments) and all the other
    >> threads do
    >> a "gd = new Global()" to get their own copy of the class (this should be
    >> done by the threads after initialization was done by main).

    >
    > This is probably the immediate cause of your problems. Each
    > thread gets its own unique instance of Global to have and to hold.
    > If forty threads all do this, there will be forty distinct Global
    > objects floating around in the program. Make sure you understand
    > that, and then read on ...
    >
    >> The idea was
    >> that since all the variables are saved in the class as "static" the other
    >> threads would be able to access any variables saved in the global class
    >> without parent class passing down a pointer to the global class
    >> reference.

    >
    > Here's the second point to ponder: No matter how many instances
    > of Global there are, there will be one and only one of each of the
    > Global class' static variables. If Global has `static short shrift;'
    > as a member, there will be exactly one Global.shrift even if there
    > are forty different Global instances. (There will be exactly one
    > Global.shrift even if there are *zero* Global instances, assuming
    > the Global class has been loaded at all.)
    >
    > Note the discrepancy: Forty Global instances, but only one
    > Global.shrift. That's not necessarily a problem, but in this
    > case ... Keep reading.
    >
    >> I have found however that is design is not work correctly because one or
    >> more threads are setting the same variables at that same time or
    >> setting/getting a variable at the same time in the global class.
    >>
    >> The original coder added while loop (see below) for the members that
    >> he want
    >> to be synchronized using a static locking flag, but in the debugger
    >> (and in
    >> real life) I can see both threads can see active as false and the
    >> while loop
    >> fails to stop one of the threads from executing the members code while
    >> the
    >> other is doing it!
    >>
    >> static boolean active = false;
    >>
    >> void synchronized connect() {
    >> while (active) {
    >> wait(50);
    >> }
    >> active = true;
    >> member code
    >> active = false;
    >> }

    >
    > (I'm assuming that both `active' and `connect' are in the
    > Global class.)
    >
    > Question: How many different threads can execute connect()
    > simultaneously? Well, how many different Global objects are
    > there that a thread might choose to synchronize on? The first
    > thread synchronizes on its own Global object, acquires the object's
    > lock and plunges into connect(). Meanwhile, the second thread
    > synchronizes on its Global object, acquires that object's lock,
    > and also plunges into connect(). And the third, and fourth, and
    > although it might be something of an unlikely coincidence you'll
    > find that, yes, all forty threads could be executing connect()
    > simultaneously, each having locked its very own Global instance.
    >
    > Question II: What have the forty threads done to stay out
    > of each other's way while mucking with the single Global.active
    > variable, the one that belongs to the whole Global class rather
    > than to any particular Global instance? They've done absolutely
    > nothing at all, that's what. So if all forty threads enter
    > connect() at the same time, many of them will execute the `while'
    > and see that active is false, and all of those will proceed
    > merrily to set it to true while they execute the `member code'.
    >
    > On the evidence available, it seems the "original coder" you
    > mention did not understand synchronization. Instead of using one
    > traffic light to control access to a busy intersection, he's given
    > every car its own private light to consult.
    >
    >> To try and fix the problem I removed the while loop and adding
    >> synchronization to one of the member declarations, but that didn't work
    >> either!

    >
    > By now, I hope you can see why.
    >
    >> I can see the main and one of the child threads calling the
    >> initializing a socket connect member at the same time even after I
    >> added the
    >> synchronization to the function declaration.

    >
    > Ah, `synchronized' was yours? Perhaps I begin to get a glimmer
    > of the original coder's motivation: He may have heard, somewhere,
    > that "synchronizing is slow" and tried to avoid it. And he wasn't
    > bright enough to realize that by doing so he created a problem that
    > not even the addition of `synchronized' can cure. (But read on ...)
    >
    >> I assume that synchronize will
    >> only work for the same class reference and not between two copies on the
    >> same class. I really don't want to recode the application and pass the
    >> global class reference down to every thread/class that needs to call
    >> global
    >> so that synchronization will work, but it looks like that's what I am
    >> going
    >> to have to do to get synchronize to work.

    >
    > It's not at all clear what you mean by "work." The `synchronized'
    > keyword "works" as designed, but I have a suspicion that you don't
    > understand the design.
    >
    >> Any ideas on how to fix this design problem without a major rewrite?

    >
    > It's easy enough the fix the immediate problem. The thing you're
    > trying to control access to ("protect") with synchronization is the
    > collection of static variables belonging to the Global class. You
    > can only protect them if all threads agree not to touch them except
    > when synchronized on the very same object. It can be any object you
    > like, as long as all the threads can find it and synchronize on it;
    > for reasons of convenience and convention, a good candidate would be
    > the Class object for the Global class itself, Global.class.
    >
    > How to synchronize on Global.class? You could do it explicitly:
    >
    > void connect() {
    > synchronized(Global.class) {
    > method code
    > }
    > }
    >
    > .... but there's an even more convenient way:
    >
    > synchronized static void connect() {
    > method code
    > }
    >
    > What's the difference between this and the original (aside from the
    > jettisoning of that silly `active' boolean and the useless cruft
    > that accompanied it)? In this version, connect() is a static method
    > rather than an instance method. So `synchronized' doesn't mean to
    > acquire the lock on `this' object -- there is no `this' for a static
    > method -- but to acquire the lock on the Class object. Bingo!
    > Problem solved.
    >
    > ... except that your grasp of synchronization seems so tenuous
    > at this point that I fear this may merely peel one layer off the
    > onion. For example: Have you noticed that the `new Global()' in
    > every thread is completely useless? If not, you haven't quite
    > understood things yet, and I foresee further difficulties. You
    > really, really need some education before trying to proceed; even
    > in Java, it takes discipline to write correct threaded code. You
    > are unlikely to get there just by trying things at random and hoping
    > for the best; even if you "get there," you won't have a reliable way
    > of knowing you've made it! The Java Tutorial has some material on
    > threading that may help, and any good textbook should explain it;
    > I strongly advise you to hit the books before coding yourself into
    > confusion. Good luck!
    >



    Hi Eric,

    Just wanted to say thanks as you've just helped me figure out what was
    wrong with my own code while reading yuor excellent description above.


    Cheers,
    Andy.
    andyb, Nov 27, 2006
    #5
  6. John T. Kerich

    Eric Sosman Guest

    Re: synchronization is not working right between two copies of thesame class for member function

    wrote:

    > I think Eric Sosman that you should re-read the post since I thought I
    > made it clear that I already know that the while loop code the previous
    > coder wrote did not work and I was trying to get synchronization to
    > work with the existing class as coded. As for the new Global being
    > "completely useless" you missed the point that the class reference was
    > not passed into any of the threads [...]


    Doesn't matter. Each thread (according to your original post)
    performs "gd = new Global()". It follows that they all have wired-
    in knowledge of the name of the Global class, and could use that name
    to synchronize explicitly on the Global.class object, or to call
    static methods (synchronized or not) of the Global class, or to refer
    to static variables of the Global class. There was nothing in your
    message to indicate that the actual instances of Global served any
    useful purpose.

    > As for making synchronization work by making the member static,
    > "synchronized static void", I was thinking that was the way to go over
    > the weekend, so thanks to Patricia and Eric for confirming that's the
    > way to fix this problem.


    Well, good luck! I still fear this is only the onion's outermost
    layer, though.

    --
    Eric Sosman
    lid
    Eric Sosman, Nov 27, 2006
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. E11
    Replies:
    1
    Views:
    4,711
    Thomas Weidenfeller
    Oct 12, 2005
  2. James.D
    Replies:
    6
    Views:
    395
    James.D
    Mar 5, 2004
  3. Replies:
    12
    Views:
    710
    fluden
    Feb 11, 2005
  4. Praetorian
    Replies:
    11
    Views:
    2,333
    James Kanze
    Apr 3, 2008
  5. Joy Beeson
    Replies:
    7
    Views:
    294
    Denis McMahon
    May 29, 2013
Loading...

Share This Page