Thanks Ivan.
David
On 12/08/2013 10:33 PM, Ivan Gerasimov wrote:
David, Chris,
I reverted back NULL-checking.
Now the change consists of one line removal and a regression test.
Webrev: http://cr.openjdk.java.net/~igerasim/8022584/6/webrev/
Hg export:
http://cr.openjdk.java.net/~igerasim/2comm
On 09/08/2013 13:18, David Holmes wrote:
I agree. I'm sure when Alan suggested to check the return he didn't
expect it to unravel like this :) As we know hotspot will never
actually return NULL there is no urgency to add this in.
Sorry about this, I wasn't aware of the issue in the JNI spec, w
Thank you Chris!
On 12.08.2013 16:43, Chris Hegarty wrote:
Thank you Ivan. This looks good to me.
-Chris.
P.S. I will give others a chance to comment. If no objections, I will
push this tomorrow for you.
On 12/08/2013 13:33, Ivan Gerasimov wrote:
David, Chris,
I reverted back NULL-checkin
Thank you Ivan. This looks good to me.
-Chris.
P.S. I will give others a chance to comment. If no objections, I will
push this tomorrow for you.
On 12/08/2013 13:33, Ivan Gerasimov wrote:
David, Chris,
I reverted back NULL-checking.
Now the change consists of one line removal and a regressi
David, Chris,
I reverted back NULL-checking.
Now the change consists of one line removal and a regression test.
Webrev: http://cr.openjdk.java.net/~igerasim/8022584/6/webrev/
Hg export:
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours
On the regression testcase theme, it got me thinking as to whether we've
any java APIs which can query what the native heap usage of the JVM. Is
that available over JMX ?
I tried the MBeanServer approach but didn't get the expected results.
Queried the "NonHeapMemoryUsage" object.
something
Hi Chris,
On 9/08/2013 8:36 PM, Chris Hegarty wrote:
Firstly, I think the memory leak issue should be moved forward
separately to this cleanup effort. They are unrelated, and I'm starting
to get the feeling that this could take some time to reach conclusion.
It seems reasonable to separate the i
Firstly, I think the memory leak issue should be moved forward
separately to this cleanup effort. They are unrelated, and I'm starting
to get the feeling that this could take some time to reach conclusion.
It seems reasonable to separate the issues.
On 09/08/2013 10:27, Ivan Gerasimov wrote:
Chris,
I would use this
if ((name_utf = (*env)->GetStringUTFChars(env, name, &isCopy)) == NULL) {
JNU_ThrowOutOfMemoryError(env, "GetStringUTFChars failed");
return NULL/JNI_False/-1;
}
If I understand it correctly, JNU_ThrowOutOfMemoryError throws an
exception only if it hasn't been a
On 09/08/2013 06:47, David Holmes wrote:
Sorry I messed this up. The JNI book says GetStringUTFChars will return
NULL and post OOME but the JNI spec (latest version 6.0) does not - it
only says it will return NULL on failure.
This is indeed strange. Most usages of this function in the jdk expec
Sorry I messed this up. The JNI book says GetStringUTFChars will return
NULL and post OOME but the JNI spec (latest version 6.0) does not - it
only says it will return NULL on failure.
So your previous version was the more correct. Given we just failed to
allocate C-heap I think we are on thin
Thumbs up!
Thanks,
David
On 9/08/2013 8:19 AM, Ivan Gerasimov wrote:
Thanks David!
On 09.08.2013 1:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Yes, it's a good point.
Since there may be a memory leak in the te
Thank you Michael!
On 09.08.2013 2:19, Michael McMahon wrote:
Ivan,
Right, it's not worth trying to do the equivalent, whatever it is, for
Windows.
The test is fine with me.
Thanks
Michael
On 08/08/13 23:15, Ivan Gerasimov wrote:
Michael,
The test uses /proc/self/stat file to detect chang
Ivan,
Right, it's not worth trying to do the equivalent, whatever it is, for
Windows.
The test is fine with me.
Thanks
Michael
On 08/08/13 23:15, Ivan Gerasimov wrote:
Michael,
The test uses /proc/self/stat file to detect changes in virtual memory
usage.
This approach is specific for Linu
Thanks David!
On 09.08.2013 1:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Yes, it's a good point.
Since there may be a memory leak in the test, it'd better not interfere
with other tests in jtreg.
Also this:
Michael,
The test uses /proc/self/stat file to detect changes in virtual memory
usage.
This approach is specific for Linux.
That's why I included the check of OS in the test.
Sincerely yours,
Ivan
On 09.08.2013 1:38, Michael McMahon wrote:
Yes, definitely "othervm" would be required for the t
Yes, definitely "othervm" would be required for the test. Also, why skip
other OS'es?
The fix is only for Linux, but it might catch future leaks on Windows.
The trouble with tests like this, is they sometimes don't age well.
Future changes in OS kernel behavior could cause problems but I guess
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50,
Thank you Michael!
I'm working on the test.
Chris, if it's not too late, I would like to include a regtest into the
change.
It will be ready in a few minutes and I'll send an updated webrev.
Thanks,
Ivan
On 08.08.2013 17:51, Michael McMahon wrote:
The patch looks good to me. I guess a regre
On 08/08/2013 06:39, Ivan Gerasimov wrote:
Thanks, David
I've updated the webrev
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/.
Thanks for fixing the other GetStringUTFChars usages too. This version
looks good to me.
-Alan.
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memle
The patch looks good to me. I guess a regression test isn't feasible.
So, the bug will be tagged noreg-hard
Michael
On 08/08/13 14:39, Ivan Gerasimov wrote:
Thanks, David
I've updated the webrev
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/.
On 08.08.2013 9:01, David Holmes wrote:
Thanks, David
I've updated the webrev
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/.
On 08.08.2013 9:01, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8 for you.
Please post an update webrev to cr.openjdk.java.net.
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an OOME
shoul
David, Alan,
I added checking for NULL results and throwing OOMException if necessary.
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.
On 07/08/2013 18:20, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return NULL.
For allocation of the result string it calls AllocateHeap() with the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot return
NULL.
For allocation of the result string it calls AllocateHeap() with the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot return
NULL.
For allocation of the result string it calls AllocateHeap() with the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops VM.
Sincerely yours,
Ivan
On 08.08.
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are there) is to
check the return from GetStringUTFChars so that the name returns when
it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class wer
32 matches
Mail list logo
