Mark Brouwer wrote:
Hi Vinod,
Thomas Vinod Johnson wrote:
Mark Brouwer wrote:
Hi all,
Attached the patch for RIVER-17 and RIVER-18, I'm sorry for the
grouping
but due to historical reasons this is the easiest way with the smallest
risk of failure to create the patch for me.
For RIVER-18, I don't recall the discussions surrounding it, and the
pointer in the report appears to point to nowhere. The change does
not seem problematic to me, but I want to make sure I understand the
motivations.
Strange, have a look at
http://mail-archives.apache.org/mod_mbox/incubator-river-dev/200703.mbox/browser
and you will find many missing mails and also subject lines that are
not a link. I'm sure that when entering the link in JIRA there was an
archived message. I don't like the archives we have, but I hope that
this is something that can be fixed ...
Original posting:
"While performing some tests for secure discovery I noticed that unicast
discovery started to fail on Java SE 6 while it worked for J2SE 1.4.2
and J2SE 5.0.
To make a very long story of troubleshooting very short it turned out
that due to the way the discovery related classes deal with the order in
which they use the unicast discovery providers the behavior can differ
between JVM. One can argue whether this is a problem or not, but at
least it is *very* confusing, it did cost me a lot of time (in which I
gained a lot of knowledge, so it wasn't completely waisted) and in this
case it turned out that while unicast discovery could have happened, it
didn't (which I think is bad).
The constraints effective [1] for the test were so that both the
plain-text and ssl providers were selected, however on J2SE 1.4.2 and
5.0 was the order { plaintext, ssl } and for Java SE 6 { ssl, kerberos
}. I tested with the Sun JVM on Windows and Solaris x86.
I could track down the problem to DiscoveryV2.doUnicastDiscovery where
an HashSet is used for the local variable 'fids' to store the discovery
formats that match the constraints effective. Changing the HashSet into
LinkedHashSet resulted in a predictable iteration order and the behavior
is now consistent for all versions of Java.
However this shows to me a bigger problem and that is which discovery
provider to use as preference. The small change from HashSet to
LinkedHashSet provides predictability but doesn't help much in which
provider to select first. That order seems to be determined mainly by
the way the service providers at the client side are found through
com.sun.jini.resource.Service, which boils down to the semantics for a
TreeSet.
My question is whether there is a strong rationale behind the TreeSet,
because my gut feeling would say that a LinkedHashSet would have been
better here to, this way the order in which the discovery providers are
stored in the
META-INF/services/com.sun.jini.discovery.DiscoveryFormatProvider file(s)
would have been leading. In which case I could have put PlainText on the
first spot as that one is likely to work under all conditions.
[1] the constraints required client and server authentication to be NO,
and that means that anonymous SSL could have worked, hence the selection
of the SSL provider. But the lookup server was not able to support
anonymous SSL because a server principal was selected due to what I
would call limitations in the ability to configure some parts of the
server side unicast discovery part but that I'll post in a different
mail.
[2] for multicast discovery all providers that pass the constraints
checks are used to send multicast requests or announcements. With
unicast discovery you can only get one shot as a client, i.e. even while
you might have multiple unicast discovery providers it is up to the
lookup server to decide which one it will use and it is proven it can
make a bad decision. My question is why doesn't the unicast discovery
response for version 2 contain alternative format IDs, besides the one
chosen by the lookup server. This would have given the client the
ability to try out other unicast discovery requests with a single format
ID in case the one chose by the server fails."
Bob's response is here
http://mail-archives.apache.org/mod_mbox/incubator-river-dev/200703.mbox/[EMAIL PROTECTED]
Thanks for digging it up.
Also you appear to have added some extra logging not covered by 17 or
18, though that's fine by me.
Additional logging is likely part of RIVER-20, but the ones that might
cause some discussion are not in this class.
I hope this explains things a bit, thanks for looking into the patch
Vinod.
Yes it's all coming back to me in bits and pieces :) I have no issues
with your changes.