Hi Chris,
Some comments,
BTW, there is a newer version of Webrev that provides convenient next
and prev links...
* sun/rmi/server/Activation.java: 1973
- I'd stick to the normal set of values for a boolean system property:
use java.lang.Boolean.getProperty("sun.rmi....").
* With so many dependencies, how about adding the modules = to an
appropriate TEST.properties file.
* With the extra security permissions and breaking into package private
utilities
this test is going to be more fragile and harder to work with.
*
test/java/rmi/activation/Activatable/checkAnnotations/CheckAnnotations.java
- line 184, leftover println... (or incorrectly indented).
- line 226 ditto?
* test/java/rmi/activation/Activatable/extLoadedImpl/ExtLoadedImplTest.java
+
test/java/rmi/activation/ActivateFailedException/activateFails/ActivateFails.java
+
test/java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java
+
test/java/rmi/activation/ActivationSystem/activeGroup/IdempotentActiveGroup.java
+
test/java/rmi/activation/ActivationSystem/modifyDescriptor/ModifyDescriptor.java
+
test/java/rmi/activation/ActivationSystem/stubClassesPermitted/StubClassesPermitted.java
+
test/java/rmi/activation/ActivationSystem/unregisterGroup/UnregisterGroup.java
+ test/java/rmi/activation/CommandEnvironment/SetChildEnv.java
+
test/java/rmi/activation/rmidViaInheritedChannel/InheritedChannelNotServerSocket.java
+
test/java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java
+
- Do these tests also need @module java.base/sun.nio.ch
* I wonder of the security.policy and rmid.security.policy files can be
(mostly) shared.
(but that's not really this issue)
* test/java/rmi/testlibrary/RMID.java
- 139: typo "do no" -> "do not"
I'm vaguely not very comfortable with scraping the port number off stdout
and the inherited channel pieces seem like a lot of moving parts.
Roger
p.s. Anyother idea
I assume not all platforms can allow separate processes to open server
sockets to the same port.
If so, we would just have the client allocate a port (0), mark it
non-exclusive and keep it open
while passing the port number to RMID. Only after RMID is started close
the allocating socket.
On 10/3/2016 11:42 AM, Chris Hegarty wrote:
Here is an updated version of this ( ready for review ):
http://cr.openjdk.java.net/~chegar/8085192_webrev.02/
Changes from previous:
1) Updated Activation/rmid to NOT redirect stderr, if an
implementation specific system property is used ( we can
discuss the name )
2) For now, I added createRMIDOnEphemeralPort, rather than
change the current implementation, as it is being used in
other places. We can revert this once other usages are
updated and verified.
-Chris.
On 29/09/16 20:09, Chris Hegarty wrote:
On 29 Sep 2016, at 16:25, Chris Hegarty <[email protected]>
wrote:
I have asked Hamlin to hold off on this for a day or so. I have an
alternative proposal that eliminates the free port anti-pattern.
It is possible to use the inheritedChannel mechanism to have the rmid
process create the server channel on an ephemeral port and report it
back to the test, i.e. remove the free port pattern.
http://cr.openjdk.java.net/~chegar/8085192_webrev/
1) The port number is reported from rmid to the test over stdout.
2) All tests pass except CheckAnnotations.java, which looks for stderr
( see 3 below ). I think the stderr check can be removed, and the
just check stdout.
3) rmid, when using inheritChannel, redirects stderr to a tmp file, so
it is not possible to get the processes stderr over the processes
stderr pipe. I did’t find that this was an issue when testing (
other
than having to clear out /tmp )
4) This could be expanded to other tests, outside of activation, to
remove more usages of free port.
This is not yet complete, I just want to share the idea to see if it
is a
runner, or not.
-Chris.
