twitter / util

Wonderful reusable code from Twitter
https://twitter.github.io/util
Apache License 2.0
2.69k stars 581 forks source link

util-jvm: Fix invocation of GarbageCollectionNotificationInfo.from() #243

Closed takezoe closed 5 years ago

takezoe commented 5 years ago

Problem

Currently, Hotspot.gcFromNotificationInfo() causes a following exception:

java.lang.IllegalArgumentException: wrong number of arguments
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.twitter.jvm.Hotspot$.gcFromNotificationInfo(Hotspot.scala:260)
    at com.twitter.jvm.Hotspot$$anon$1.handleNotification(Hotspot.scala:95)
    at sun.management.NotificationEmitterSupport.sendNotification(NotificationEmitterSupport.java:156)
    at sun.management.GarbageCollectorImpl.createGCNotification(GarbageCollectorImpl.java:143)

Solution

Give null as the receiver of invocation of GarbageCollectionNotificationInfo.from() which uses reflection because this method is static.

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

codecov-io commented 5 years ago

Codecov Report

Merging #243 into develop will increase coverage by <.01%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #243      +/-   ##
===========================================
+ Coverage    47.12%   47.13%   +<.01%     
===========================================
  Files          231      231              
  Lines        14826    14826              
  Branches      1184     1184              
===========================================
+ Hits          6987     6988       +1     
+ Misses        7839     7838       -1
Impacted Files Coverage Δ
...l-jvm/src/main/scala/com/twitter/jvm/Hotspot.scala 14.14% <0%> (ø) :arrow_up:
...in/scala/com/twitter/logging/QueueingHandler.scala 93.93% <0%> (-6.07%) :arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala 77.08% <0%> (-0.42%) :arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala 91.78% <0%> (+0.42%) :arrow_up:
...ng/src/main/scala/com/twitter/logging/Logger.scala 64.92% <0%> (+0.74%) :arrow_up:
...main/scala/com/twitter/concurrent/AsyncQueue.scala 98.52% <0%> (+1.47%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0d9dded...1b46096. Read the comment docs.

xerial commented 5 years ago

This error message is also shown a lot when running the tests in Finatra.

kevinoliver commented 5 years ago

Apologies! This was a bug that I introduced then reverted then patched again, within a day.

Our internal code that pushes changes out to Github botched the revert and the follow-up patch. As such, this is broken in the 19.3.0 release.

I've pushed the fix (https://github.com/twitter/util/commit/52d49ad98c3ec3b08218a60d6ede322c0f7edc5e) and it will be released in 19.4.0.

In the meantime, this is a logging issue and can be turned off by setting the log levels appropriately.