On Thu, 28 Jan 2021 03:39:37 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> It looks like >> [JDK-8215622](https://bugs.openjdk.java.net/browse/JDK-8215622) introduced a >> 4th argument to the attach API, and this caused hangs when and older JVMs >> tried to attach. That was fixed by >> [JDK-8219721](https://bugs.openjdk.java.net/browse/JDK-8219721), and seemed >> to revert the arg count back to 3, although it's not too clear to me how >> this was accomplished. The webrev is here: >> >> http://cr.openjdk.java.net/~xiaofeya/8219721/webrev.01/ >> >> Also at the same time the following was filed, which has not yet been >> addressed: >> >> [JDK-8219896](https://bugs.openjdk.java.net/browse/JDK-8219896) Parameter >> size mismatch between client and VM sides of the Attach API >> >> @linzang JDK-8215622 and JDK-8219721 were your CRs, so maybe you can >> elaborate on them a bit more. > > Dear @plummercj， > You are right, the same issue has been encountered when I was developing > JDK-8215622. At that time the argument number does not actully exceed the > limitation (3), and so I made a quick fix to change back arg_count_max to 3. > And the reason that I originally change the arg_count_max is I was trying to > add more options to jmap, and finally only "file" and "parallel" were > accepted, so the arg_count_max never exceed the limitation. > > Then it comes to this patch, with "parallel" added to jmap -dump, there can > be 4 arguments and we need to handle the arg count limitation. Here are the > experiments I have been tried recently (and also previously when handling > JDK-8219721): > > - Enlarge arg_count_max from 3 to 4 > > This cause the same issue as describe by JDK-8219721, when using an old > jcmd tools to communicate with new JDK, it hangs, which is a huge > compatibitily issue, so we need to fix it. > > - Let jmap pass all arguments as a whole one, just like what jcmd does, and > modify attachlistener.cpp to parse arguments from op->arg(0), and also > distinguish old/new jmap by testing whether op->arg(1) is null. > > This works well until the new jmap is used to communicate with old JDK. In > this case, the old JDK doesn't parse op->arg(0), which is passed by jmap as a > string containing all arguments. So all parameters are treated as dump > filename incorrectly. > > - Let jmap still passing three arguents, and combine the "gz" and "parallel" > together as a string and pass it to JDK, and modify attachlistener.cpp to > parse the 3rd arguments from op->arg(2). > > This works well for old/new jmap targeting on old/new JDK when "parallel" > and "gz" options are not used together. However, it has the issue when new > jmap targeting to old JDK and both "parallel" and "gz" are used. because old > JDK > can not parse op->arg(3), so the "gz" option will not work. Although I don't > think it is reasonable to use new jmap with "parallel" option on an old JDK, > but it is possible, and the issue is that old JDK will not prompt any error > message in this case, it just dump the heap by ignoring the "gz" value. > > - I also worked out a patch using timeout mechanism in socket. > > This solution seems work, so the arg_count_max could be enlarged to 4, and > when old jmap is targeting on new JDK, the socket can be timeout on waiting > for the 4th arguments while old jmap only provide 3. And then it could work > as expected. But this seems need to change the socket for different > platform, and I just verified it on Linux, I am not sure whether it is > acceptable. > > - I am also trying to see whether there can be other solution that mentioned > in the previous discussion thread. ( links here: > https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-February/027240.html, > FYI) > > BTW, Do you think we need to solve this issue seperately? Maybe have a patch > for JDK-8219896? And when it is fixed, appling the parallel dump patch on > top of that? > > Thanks, > Lin Hi Lin, It is also in my memory that you actually did not have 4 arguments. The real incompatibility issue was that the order of arguments was swapped. It is why it was relatively easy to fall back and just update the constants with 3 instead of 4 and swap the order of arguments back. Thanks, Serguei ------------- PR: https://git.openjdk.java.net/jdk/pull/2261
