Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-07-02 Thread Deven You

Hi All,
Could anyone notice this problem?

Thanks a lot!
On 06/25/2012 04:18 PM, Deven You wrote:

Hi All,

First of all, if the jdbc problem has a better mailing list to post 
please tell me.


I find that javax.sql.rowset.serial.SerialBlob is not fully 
implemented in OpenJDK 8. Methods


public InputStream getBinaryStream(long pos,long length) throws 
SQLException

public void free() throws SQLException

only throw UnsupportedOperationException.

I have made a patch[1] to implement these 2 methods. Could anyone take 
a look to review it.


BTW: I think the spec for SerialBlob is not very clear like it doesn't 
mention if all method rather than free() need throw any exception 
after free() is invoked. However that behavior seems reasonable.


[1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev

Thanks a lot.




--
Best Regards,

Deven



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-07-02 Thread Lance Andersen - Oracle
Hi Deven,

Thanks for the email and the proposed patch.  I will look at this later today 
or tomorrow.  I actually have made these changes in my workspace for JDK 8 but 
will compare your changes to mine.

Best
Lance
On Jul 2, 2012, at 5:04 AM, Deven You wrote:

 Hi All,
 Could anyone notice this problem?
 
 Thanks a lot!
 On 06/25/2012 04:18 PM, Deven You wrote:
 Hi All,
 
 First of all, if the jdbc problem has a better mailing list to post please 
 tell me.
 
 I find that javax.sql.rowset.serial.SerialBlob is not fully implemented in 
 OpenJDK 8. Methods
 
public InputStream getBinaryStream(long pos,long length) throws 
 SQLException
public void free() throws SQLException
 
 only throw UnsupportedOperationException.
 
 I have made a patch[1] to implement these 2 methods. Could anyone take a 
 look to review it.
 
 BTW: I think the spec for SerialBlob is not very clear like it doesn't 
 mention if all method rather than free() need throw any exception after 
 free() is invoked. However that behavior seems reasonable.
 
 [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev
 
 Thanks a lot.
 
 
 
 -- 
 Best Regards,
 
 Deven
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: [8] Review request for 7162111 change tests run in headless mode [macosx]

2012-07-02 Thread Anthony Petrov

Looks fine to me.

--
best regards,
Anthony

On 07/02/12 18:18, Jason Uh wrote:

Anthony and Alan,

Thanks for your comments. I've reverted the changes to CommonSetup.sh so
that XToolkit is no longer forced. Tests still pass.

Please see the new webrev:
http://cr.openjdk.java.net/~juh/7162111/webrev.01/

Thanks,
Jason

On 06/25/2012 06:19 AM, Anthony Petrov wrote:

Hi Alan and Jason,

On 06/23/12 11:28, Alan Bateman wrote:

On 23/06/2012 02:01, Jason Uh wrote:

This fix was for regression tests failing on Mac OS X on remotely
executed environments. The changed tests now run in headless mode and
have been taken off the Problem List.

Webrev: http://cr.openjdk.java.net/~juh/7162111/webrev.00/
The CR: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7162111

Note that test/demo/jvmti/mtrace/TraceJFrame.java was not fixed here
because headless mode is not supported for JFrame. A separate CR will
be created for this.


It's good to see these tests changed to run headless and will make the
test execution much more reliable. Aside from the mtrace demo there are
a couple of other tests that periodically hang initializing AWT, at
least when running via ssh and then depending on whether someone is
logged in and other configuration settings. Some of the shell tests for
serialization come to mind (BTW: no problem doing that via a separate
bug, just mentioning that there are other tests that are problems too).

One question, and this may be a question for Artem or others, is that in
CommonSetup.sh you set AWT_TOOLKIT=XToolkit. Is that right?


I don't think we need to force XToolkit on the Mac. We don't quite
support it on that platform actually. The normal headless CToolkit
should work just fine. Could you please revert the changes to
CommonSetup.sh and verify if the tests pass?

--
best regards,
Anthony


hg: jdk8/tl/jdk: 7174887: Deadlock in jndi ldap connection cleanup

2012-07-02 Thread rob . mckenna
Changeset: ecc5dd3790a1
Author:robm
Date:  2012-07-02 19:32 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ecc5dd3790a1

7174887: Deadlock in jndi ldap connection cleanup
Reviewed-by: xuelei

! src/share/classes/com/sun/jndi/ldap/Connection.java
! src/share/classes/com/sun/jndi/ldap/LdapClient.java



Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread Stuart Marks

On 7/1/12 5:39 PM, David Holmes wrote:

Now, how can the test fail? If ref is never cleared, the while loop will
never terminate, and we rely on jtreg to timeout and kill this test. It
would be good to have a brief comment above the while loop that explains
this.


Agreed. Something like

// Might require multiple calls to System.gc() for weak-references processing
to be complete.
// If the weak-reference is not cleared as expected we will hang here
// until timed out by the test harness

Though now I write that I'm unhappy that this will only behave nicely if run
from jtreg. We do use explicit timeouts in other tests.


So, are you unhappy enough that we should ask Eric to add an explicit timeout?

I don't think it would be that difficult. Perhaps a technique like the 
following would suffice. Get System.currentTimeMillis() before the loop, and 
call it again each time through the loop, and fail if the difference is greater 
than the timeout (e.g., 60 sec). Doesn't involve other threads or even 
Timer/TimerTask.


On the other hand if one is running this test directly instead of through 
jtreg, one can just ^C it if it's taking an unexpectedly long time.


s'marks


hg: jdk8/tl/jdk: 7176907: additional warnings cleanup in java.util, java.util.regexp, java.util.zip

2012-07-02 Thread stuart . marks
Changeset: b2fc66012451
Author:smarks
Date:  2012-07-02 14:11 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b2fc66012451

7176907: additional warnings cleanup in java.util, java.util.regexp, 
java.util.zip
Reviewed-by: forax, khazra, smarks
Contributed-by: Mani Sarkar sadhak...@gmail.com

! src/share/classes/java/util/InvalidPropertiesFormatException.java
! src/share/classes/java/util/regex/Pattern.java
! src/share/classes/java/util/zip/GZIPInputStream.java



Re: Bug 7176907 - Patches for javac warnings cleanup (text and util) from Adopt OpenJDK

2012-07-02 Thread Stuart Marks

Pushed:

http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b2fc66012451

Thanks for your contributions to OpenJDK!

s'marks

On 7/1/12 4:05 AM, Martijn Verburg wrote:

Hi Stuart,


snip

The java.util patches look good and are almost ready to go in. The only
issue is how to format the Contributed-by line in the changeset comment.
What I have so far for the comment is:

7176907: additional warnings cleanup in java.util, java.util.regexp,
java.util.zip
Reviewed-by: forax, khazra, smarks
Contributed-by: ???

The contributed-by line takes one or more email addresses or email/name
pairs. For an earlier contribution from LJC, see here [1]. This isn't a
terribly big deal but I want to make sure that credit goes where it's due.
Can you tell me what I should put for the contributed-by line?


Main Sarkar - sadhak001ATgmail.DOTcom


The warnings in java.text have already been fixed by Deepak Bhole's
changeset [2]. Not a problem, this took two minutes to figure out.


Cool, we've adjusted our workflow to take this into account.


There were a couple questions from earlier in the thread.

On the discussion of when the compiler issues switch fallthrough warnings,
from what I can tell, the compiler issues a warning whenever there is actual
code in a case that doesn't end with break, continue, return, or throw. This
seems independent of whether what follows is another case or the 'default'
case. If there are several case clauses together with no intervening code,
this isn't considered a fallthrough and thus there is no warning. This make
sense, as sometimes you want several different cases to be handled by the
same code. For example,

 switch (ch) {
 case 'a':
 // no warning here
 case 'b':
 someActualWork();
 break;
 // ...
 }


Understood, made sense to us as well.


Regarding whether there is a style checker for indentation and spacing, I'm
not aware of a good one that I can recommend. We generally adhere to the
(very old) Java Coding Conventions [3]. I think most people just deal with
style issues manually by hand and by eye; I know I do. We do run jcheck [4]
on every incoming changeset, but the only things it checks in files'
contents are for trailing whitespace, and CR (as opposed to LF) and TAB
characters.


OK, we've added jcheck to the list of things to run going forwards.
Perhaps in the
future one of our folks can put together a checkstyle ruleset for
core-lib (controversial!)...
:-)

Thanks for getting this patch merged!


Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread David Holmes

On 3/07/2012 7:08 AM, Stuart Marks wrote:

On 7/1/12 5:39 PM, David Holmes wrote:

Now, how can the test fail? If ref is never cleared, the while loop will
never terminate, and we rely on jtreg to timeout and kill this test. It
would be good to have a brief comment above the while loop that explains
this.


Agreed. Something like

// Might require multiple calls to System.gc() for weak-references
processing
to be complete.
// If the weak-reference is not cleared as expected we will hang here
// until timed out by the test harness

Though now I write that I'm unhappy that this will only behave nicely
if run
from jtreg. We do use explicit timeouts in other tests.


So, are you unhappy enough that we should ask Eric to add an explicit
timeout?


No. But I'm not the authoritive voice in this area :)


I don't think it would be that difficult. Perhaps a technique like the
following would suffice. Get System.currentTimeMillis() before the loop,
and call it again each time through the loop, and fail if the difference
is greater than the timeout (e.g., 60 sec). Doesn't involve other
threads or even Timer/TimerTask.


Or simply limit the number of loops so that N*sleepTime = timeout. This 
then requires adding back the check for null outside the loop.



On the other hand if one is running this test directly instead of
through jtreg, one can just ^C it if it's taking an unexpectedly long time.


True.

David
-



s'marks


Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread Eric Wang

Hi Stuart and David,

Thanks for the suggestion about the timeout. so there are three ways to 
handle the timeout.


1. Add comment to explain that test failed due to default timeout
2. Specify the the timeout option
3. Timeout programly

Is it ok for you that i'd like to choose the second way explict 
timeout as the test looks simply and lucid?


Regards,
Eric

On 2012/7/3 10:38, David Holmes wrote:

On 3/07/2012 7:08 AM, Stuart Marks wrote:

On 7/1/12 5:39 PM, David Holmes wrote:
Now, how can the test fail? If ref is never cleared, the while loop 
will
never terminate, and we rely on jtreg to timeout and kill this 
test. It
would be good to have a brief comment above the while loop that 
explains

this.


Agreed. Something like

// Might require multiple calls to System.gc() for weak-references
processing
to be complete.
// If the weak-reference is not cleared as expected we will hang here
// until timed out by the test harness

Though now I write that I'm unhappy that this will only behave nicely
if run
from jtreg. We do use explicit timeouts in other tests.


So, are you unhappy enough that we should ask Eric to add an explicit
timeout?


No. But I'm not the authoritive voice in this area :)


I don't think it would be that difficult. Perhaps a technique like the
following would suffice. Get System.currentTimeMillis() before the loop,
and call it again each time through the loop, and fail if the difference
is greater than the timeout (e.g., 60 sec). Doesn't involve other
threads or even Timer/TimerTask.


Or simply limit the number of loops so that N*sleepTime = timeout. 
This then requires adding back the check for null outside the loop.



On the other hand if one is running this test directly instead of
through jtreg, one can just ^C it if it's taking an unexpectedly long 
time.


True.

David
-



s'marks





Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread David Holmes

Hi Eric,

On 3/07/2012 1:28 PM, Eric Wang wrote:

Hi Stuart and David,

Thanks for the suggestion about the timeout. so there are three ways to
handle the timeout.

1. Add comment to explain that test failed due to default timeout
2. Specify the the timeout option
3. Timeout programly

Is it ok for you that i'd like to choose the second way explict
timeout as the test looks simply and lucid?


If by #2 you mean add something like:

@run main/timeout=10

then no. Earlier this year there were changes to a bunch of tests that 
set timeouts shorter than the jtreg default, to delete the timeouts. 
Given that, my misgivings about relying on the harness are not relevant 
so we need only do #1 and add a comment to make it clear that a failing 
test will be indicated via the harness timeout.


David
-


Regards,
Eric

On 2012/7/3 10:38, David Holmes wrote:

On 3/07/2012 7:08 AM, Stuart Marks wrote:

On 7/1/12 5:39 PM, David Holmes wrote:

Now, how can the test fail? If ref is never cleared, the while loop
will
never terminate, and we rely on jtreg to timeout and kill this
test. It
would be good to have a brief comment above the while loop that
explains
this.


Agreed. Something like

// Might require multiple calls to System.gc() for weak-references
processing
to be complete.
// If the weak-reference is not cleared as expected we will hang here
// until timed out by the test harness

Though now I write that I'm unhappy that this will only behave nicely
if run
from jtreg. We do use explicit timeouts in other tests.


So, are you unhappy enough that we should ask Eric to add an explicit
timeout?


No. But I'm not the authoritive voice in this area :)


I don't think it would be that difficult. Perhaps a technique like the
following would suffice. Get System.currentTimeMillis() before the loop,
and call it again each time through the loop, and fail if the difference
is greater than the timeout (e.g., 60 sec). Doesn't involve other
threads or even Timer/TimerTask.


Or simply limit the number of loops so that N*sleepTime = timeout.
This then requires adding back the check for null outside the loop.


On the other hand if one is running this test directly instead of
through jtreg, one can just ^C it if it's taking an unexpectedly long
time.


True.

David
-



s'marks