{"action":"created","issue":{"url":"https://api.github.com/repos/google/allocation-instrumenter/issues/15","repository_url":"https://api.github.com/repos/google/allocation-instrumenter","labels_url":"https://api.github.com/repos/google/allocation-instrumenter/issues/15/labels{/name}","comments_url":"https://api.github.com/repos/google/allocation-instrumenter/issues/15/comments","events_url":"https://api.github.com/repos/google/allocation-instrumenter/issues/15/events","html_url":"https://github.com/google/allocation-instrumenter/issues/15","id":151182565,"number":15,"title":"NullPointerException in other shutdown hooks","user":{"login":"remkop","id":5917763,"avatar_url":"https://avatars.githubusercontent.com/u/5917763?v=3","gravatar_id":"","url":"https://api.github.com/users/remkop","html_url":"https://github.com/remkop","followers_url":"https://api.github.com/users/remkop/followers","following_url":"https://api.github.com/users/remkop/following{/other_user}","gists_url":"https://api.github.com/users/remkop/gists{/gist_id}","starred_url":"https://api.github.com/users/remkop/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/remkop/subscriptions","organizations_url":"https://api.github.com/users/remkop/orgs","repos_url":"https://api.github.com/users/remkop/repos","events_url":"https://api.github.com/users/remkop/events{/privacy}","received_events_url":"https://api.github.com/users/remkop/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2016-04-26T16:40:14Z","updated_at":"2016-05-04T17:35:11Z","closed_at":null,"body":"We are working to [make Log4j 2 garbage-free](https://issues.apache.org/jira/browse/LOG4J2-1270) and we are using the Allocation Instrumenter in our [unit tests](https://issues.apache.org/jira/browse/LOG4J2-1295) to verify that we don't allocate objects during steady state logging.\r\n\r\nWe see our tests randomly fail with this exception:\r\n\r\n```\r\nException in thread \"Thread-0\" java.lang.NullPointerException\r\n at com.google.monitoring.runtime.instrumentation.AllocationRecorder.getObjectSize(AllocationRecorder.java:200)\r\n at com.google.monitoring.runtime.instrumentation.AllocationRecorder.recordAllocation(AllocationRecorder.java:244)\r\n at java.util.Hashtable.getEnumeration(Hashtable.java:665)\r\n at java.util.Hashtable.keys(Hashtable.java:329)\r\n at java.util.logging.LogManager$LoggerContext.getLoggerNames(LogManager.java:688)\r\n at java.util.logging.LogManager.reset(LogManager.java:1135)\r\n at java.util.logging.LogManager$Cleaner.run(LogManager.java:248)\r\n```\r\n\r\nI believe this is a bug in AllocationRecorder:\r\n\r\nThe NullPointerException above occurs when the (java.util.logging) LogManager$Cleaner shutdown hook allocates a new object and the instrumented bytecode invokes the recordAllocation method.\r\n\r\nThe AllocationRecorder also has a shutdown hook which sets its instrumentation field to null.\r\nThese shutdown hook threads can run at the same time, so if the instrumentation field is set to null after the null check (line 236) but before it is used in getObjectSize (line 200), we get the above exception.\r\n\r\nOne solution would be to introduce a local Instrumentation variable, assign the value of the static instrumentation field to the local variable, and do the null check on that local variable. If non-null, call the getObjectSize method with the local Instrumentation variable as one of the parameters. The getObjectSize method would use the passed Instrumentation parameter and should not access the static field.\r\n"},"comment":{"url":"https://api.github.com/repos/google/allocation-instrumenter/issues/comments/216942209","html_url":"https://github.com/google/allocation-instrumenter/issues/15#issuecomment-216942209","issue_url":"https://api.github.com/repos/google/allocation-instrumenter/issues/15","id":216942209,"user":{"login":"remkop","id":5917763,"avatar_url":"https://avatars.githubusercontent.com/u/5917763?v=3","gravatar_id":"","url":"https://api.github.com/users/remkop","html_url":"https://github.com/remkop","followers_url":"https://api.github.com/users/remkop/followers","following_url":"https://api.github.com/users/remkop/following{/other_user}","gists_url":"https://api.github.com/users/remkop/gists{/gist_id}","starred_url":"https://api.github.com/users/remkop/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/remkop/subscriptions","organizations_url":"https://api.github.com/users/remkop/orgs","repos_url":"https://api.github.com/users/remkop/repos","events_url":"https://api.github.com/users/remkop/events{/privacy}","received_events_url":"https://api.github.com/users/remkop/received_events","type":"User","site_admin":false},"created_at":"2016-05-04T17:35:11Z","updated_at":"2016-05-04T17:35:11Z","body":"I can create another patch. For what it's worth, there is also this alternative: \r\n\r\n```\r\npublic class AllocationRecorder {\r\n  private static final Instrumentation NULL_INSTRUMENTATION = (Instrumentation) Proxy.newProxyInstance(\r\n          AllocationRecorder.class.getClassLoader(), new Class[]{Instrumentation.class}, new InvocationHandler() {\r\n            @Override\r\n            public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {\r\n              if (\"getObjectSize\".equals(method.getName())) {\r\n                return Long.valueOf(0);\r\n              }\r\n              throw new UnsupportedOperationException(method.getName());\r\n            }\r\n          }\r\n  );\r\n\r\n```\r\n\r\nNow the concrete NullInstrumentation class is no longer necessary."}}