Hi Lars,

Thank you for the great explanation. I'll read up on Threading and get this one 
right :))

-- Edvin

lars vonk skrev:
The iteraton is called with synchronized(tlds) { ... } around, but maybe
that's not enough? Should I wrap all calls to setXXX in TldChecker with a
synchronized, or even put synchronized directly on the getters/setters of
Tld?


It is important that the read and write operation synchronize on the same
lock. As I understand it the write of the "done" and "available" flag are in
the TldChecker class which aren't locked by the same lock (the lock on tlds)
you put around the iteration. This is because another thread is the
executing of the code of the TldChecker (that is done via the Executor, if I
understand your code correctly). So in order to make sure your write of the
"done" and "available" flag are seen by other threads you can do two things:

1. Let the different Threads synchronize on the same lock. The simplest way
is to make the setXXX and isXXX methods synchronized as you describe.
2. Declare the "done" and "available" properties as volatile.
In my recollection this is the only way that, according to the JMM,
guarantees visibility in other Threads.

Another thing is that your populateItem method contains more probable
concurrency issues:

It can happen that during the populateItem method the current Tld is under
process. That would cause some strange behavior:

For instance:


/* Link to show who owns the domain if it's taken */
item.add(new Link("taken") {
     public boolean isVisible() {
            return tld.getDone() && !tld.getAvailable() &&
!tld.getIllegal();
     }


If the above code executes while in another Thread the TldChecker is
currently processing the same tl, then at this point it can happen that
the tld.getDone() returns false, as the TldChecker is not done yet. But
then when adding the CheckBox like below:


/* Checkbox so it can be ordered */
item.add(new CheckBox("selected", new PropertyModel(tld, "selected")) {
     public boolean isVisible() {
          return (tld.getDone() && tld.getAvailable());
    }
});


The tld.getDone() can return true as it can happen that the TldChecker just
finished up with the Tld. So there is an inconsistency between the CheckBox
and the Link. I don't know the requirements of your application so I can't
see if this is bad or not.

A way to prevent this is to make sure that populateItem has the exclusive
lock for the Tld it is currently processing, so synchronize on the tld that
is in the item.getModelObject() AND make sure the TldChecker also
synchronizes on the tld it is processing.

The thing is that with these kind of Threading issues it can go right 100
times in a row and than it goes wrong for 10 times, or it may never go
wrong. If you don't use correct locking you just don't get any guarantees
from the JVM, only headaches :-).

Lars





On Mon, Apr 21, 2008 at 5:32 PM, Edvin Syse <[EMAIL PROTECTED]> wrote:

lars vonk wrote:

Hi,

How is the "done" and "available" flag set in the Tld class by the
TldChecker? Since you are using a separate Thread that sets these flags
:

/* Iterate over the tlds and instantiate a TldChecker */

for(Tld tld : tlds)
      e.execute(new TldChecker(tld, query)); // Args could be for
instance "com", and "mydomain"

The iteraton is called with synchronized(tlds) { ... } around, but maybe
that's not enough? Should I wrap all calls to setXXX in TldChecker with a
synchronized, or even put synchronized directly on the getters/setters of
Tld?

Another thing is, (I am not sure about this) is that if the
Checkbox.isVisible method returns false wouldn't Wicket always complain
since there is HTML markup but no corresponding Java component?

When you do setVisible(false) on a component, the corresponding markup
will be hidden. That's the beauty of it.

I know that getVisible() might be called multiple times during a
page-render, so I'm beginning to think that maybe it returns false on the
call that decides wether to include the markup, and then true later on,
creating the mismatch in hiearachy. Does this sound plausible?


-- Edvin

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to