Staffan, http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.03/
After couple of experiments I end up with simple fix - don't coredump on AGENT_ERROR_TRANSPORT_INIT Current code don't propagate transport error to upper level, so if we need fine grained control I'll rewrite transport initialization. -Dmitry On 2014-08-25 15:53, Staffan Larsen wrote: > > On 25 aug 2014, at 13:33, Dmitry Samersoff <dmitry.samers...@oracle.com > <mailto:dmitry.samers...@oracle.com>> wrote: > >> Staffan, >> >> On 2014-08-25 15:26, Staffan Larsen wrote: >>> Dmitry, >>> >>> One problem with this fix is that debugInit_exit() is used from many >>> places in the JDWP code. For some of these I think it still does make >>> sense to receive a core dump for analysis. I agree that when parsing >>> options, we do not need a core dump. >> >> jdwp has explicit option to request a coredump at debugInit_exit(). >> >> Is it enough or I should create a more sophisticated fix ? > > I don’t think that is enough. Often a problem will happen and will not > be reproducible. > > Maybe this code: > > if ((arg.error != JDWP_ERROR(NONE)) && > (arg.startCount == 0) && > initOnStartup) { > EXIT_ERROR(map2jvmtiError(arg.error), "No transports initialized"); > } > > can use some variant of EXIT_ERROR that does not call fatalError() ? > > /Staffan > >> >> Actually, I don't think that coredump on every call to jni_FatalError() >> (see jni.cpp:726) is necessary but this hotspot code is here for 6 years >> and changing it probably out of scope of this fix. >> >> -Dmitry >> >> >>> >>> /Staffan >>> >>> >>> On 20 aug 2014, at 17:55, Dmitry Samersoff >>> <dmitry.samers...@oracle.com <mailto:dmitry.samers...@oracle.com>> wrote: >>> >>>> Serguei, >>>> >>>> After some additional testing I changed the fix a bit. Please take >>>> a look at new version. >>>> >>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.02/ >>>> >>>> New version reports JVMTI error to stderr. >>>> >>>> Also jniFatalError is not referenced any more so I remove it. >>>> >>>> -Dmitry >>>> >>>> >>>> >>>> On 2014-08-20 12:08, serguei.spit...@oracle.com wrote: >>>>> Ok. >>>>> >>>>> Thank you for the explanation! Serguei >>>>> >>>>> On 8/20/14 1:01 AM, Dmitry Samersoff wrote: >>>>>> Serguei, >>>>>> >>>>>> 1. Historically JDI test-suite had no tests for failed >>>>>> transport initialization behavior and invalid parameters >>>>>> handling. >>>>>> >>>>>> 2. As a part of JDWP hardening work I added couple of such >>>>>> tests to OptionTest.java - these tests pass invalid parameters >>>>>> to dt_socket transport to make sure that transport doesn't >>>>>> crash (one such crash was discovered and fixed) but just return >>>>>> non-zero exit code to upper level. >>>>>> >>>>>> 3. After fix for JDK-6694099 any non-zero exit code from >>>>>> transport cause VM to coredump. Dumping multiple cores on busy >>>>>> machine takes a time so harness kills the test by timeout. >>>>>> >>>>>> We can just increase timeout for this test but I don't think >>>>>> it's a good idea to dump core when invalid parameters passed to >>>>>> transport. >>>>>> >>>>>> So there is the fix. >>>>>> >>>>>> 4. After the fix tests for negative parameters will return >>>>>> non-zero exit code as expected but will not dump the core. >>>>>> >>>>>> -Dmitry >>>>>> >>>>>> On 2014-08-20 00:54, serguei.spit...@oracle.com wrote: >>>>>>> Hi Dmitry, >>>>>>> >>>>>>> The fix seems to be Ok. Just want to make it clear... This >>>>>>> fix just changes the bug pattern. It a case of incorrect >>>>>>> transport parameters the test is still going to fail but >>>>>>> without crash, right? >>>>>>> >>>>>>> Thanks, Serguei >>>>>>> >>>>>>> On 8/19/14 12:09 PM, Dmitry Samersoff wrote: >>>>>>>> Hi Everybody, >>>>>>>> >>>>>>>> Please review the fix: >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.01/ >>>>>>>> >>>>>>>> >>>>>>>> >> JDWP call jniFatalError if transport can't be initialized (e.g. wrong >>>>>>>> parameters) and jniFatalError call os::abort(). Therefor >>>>>>>> all transport initialization errors cause vm to coredump. >>>>>>>> >>>>>>>> I see no reason for debugInit_exit to call jniFatalError so >>>>>>>> remove this code. >>>>>>>> >>>>>>>> -Dmitry >>>>>>>> >>>>>> >>>>> >>>> >>>> >>>> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, >>>> Russia * I would love to change the world, but they won't give me >>>> the sources. >>> >> >> >> -- >> Dmitry Samersoff >> Oracle Java development team, Saint Petersburg, Russia >> * I would love to change the world, but they won't give me the sources. > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
