Mercurial > hg > thermostat
changeset 2295:5d4c3090df2d
Avoid potential IOOB exception after dump-heap command completion
PR2946
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-May/018753.html
author | Andrew Azores <aazores@redhat.com> |
---|---|
date | Thu, 12 May 2016 11:17:00 -0400 |
parents | 181cffa3afd2 |
children | 7596994808a2 |
files | vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java |
diffstat | 4 files changed, 63 insertions(+), 3 deletions(-) [+] |
line wrap: on
line diff
--- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Thu May 12 09:52:34 2016 -0400 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Thu May 12 11:17:00 2016 -0400 @@ -97,8 +97,13 @@ @Override public void run() { String latestHeapId = getLatestHeapId(heapDAO, agentId, vmId); - ctx.getConsole().getOutput().println(translator.localize(LocaleResources.COMMAND_HEAP_DUMP_DONE, - latestHeapId).getContents()); + // latestHeapId may be null if last heap dump is actually not yet available in storage + if (latestHeapId != null) { + ctx.getConsole().getOutput().println(translator.localize(LocaleResources.COMMAND_HEAP_DUMP_DONE, + latestHeapId).getContents()); + } else { + ctx.getConsole().getOutput().println(translator.localize(LocaleResources.COMMAND_HEAP_DUMP_DONE_NOID).getContents()); + } s.release(); } }; @@ -124,8 +129,16 @@ } } + // FIXME: storage may actually return us outdated results which do not contain the latest + // heap dump(s). This can result in an empty list being returned (which we signal here by + // returning null), or can result in a second dump-heap command incorrectly echoing the same + // heap dump ID as the immediately prior dump-heap command. + // See discussion here: http://icedtea.classpath.org/pipermail/thermostat/2016-May/018753.html static String getLatestHeapId(HeapDAO heapDao, AgentId agentId, VmId vmId) { Collection<HeapInfo> heapInfos = heapDao.getAllHeapInfo(agentId, vmId); + if (heapInfos.isEmpty()) { + return null; + } List<HeapInfo> sortedByLatest = new ArrayList<>(heapInfos); Collections.sort(sortedByLatest, new Comparator<HeapInfo>() { @Override
--- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java Thu May 12 09:52:34 2016 -0400 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java Thu May 12 11:17:00 2016 -0400 @@ -61,6 +61,7 @@ HEAP_DUMP_ERROR, COMMAND_HEAP_DUMP_DONE, + COMMAND_HEAP_DUMP_DONE_NOID, COMMAND_FIND_ROOT_NO_ROOT_FOUND,
--- a/vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties Thu May 12 09:52:34 2016 -0400 +++ b/vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties Thu May 12 11:17:00 2016 -0400 @@ -20,6 +20,7 @@ HEAP_DUMP_ERROR = Error dumping heap (agent: {0}, vm: {1}) COMMAND_HEAP_DUMP_DONE = Heap dump ID: {0} +COMMAND_HEAP_DUMP_DONE_NOID = Heap dump completed COMMAND_FIND_ROOT_NO_ROOT_FOUND = No root found for: {0}
--- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Thu May 12 09:52:34 2016 -0400 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Thu May 12 11:17:00 2016 -0400 @@ -97,7 +97,8 @@ HeapInfo heapInfo = new HeapInfo(); heapInfo.setHeapDumpId("0001"); - when(heapDao.getAllHeapInfo(new AgentId("myAgent"), new VmId(vmInfo.getVmId()))).thenReturn(Collections.singleton(heapInfo)); + heapInfo.setHeapId("0001-0002"); + when(heapDao.getAllHeapInfo(any(AgentId.class), any(VmId.class))).thenReturn(Collections.singleton(heapInfo)); DumpHeapHelper impl = mock(DumpHeapHelper.class); final ArgumentCaptor<Runnable> successHandler = ArgumentCaptor @@ -132,6 +133,50 @@ } @Test + public void verifyEchosNoIdMessageWhenNoResultFromStorageAfterDumpCompletes() throws CommandException { + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); + AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); + HeapDAO heapDao = mock(HeapDAO.class); + RequestQueue queue = mock(RequestQueue.class); + VmInfo vmInfo = new VmInfo("myAgent", "foo", 123, 0, 0, null, null, null, null, null, null, null, null, null, + null, null,0, "myUsername"); + when(vmInfoDao.getVmInfo(new VmId("foo"))).thenReturn(vmInfo); + + when(heapDao.getAllHeapInfo(any(AgentId.class), any(VmId.class))).thenReturn(Collections.<HeapInfo>emptyList()); + + DumpHeapHelper impl = mock(DumpHeapHelper.class); + final ArgumentCaptor<Runnable> successHandler = ArgumentCaptor + .forClass(Runnable.class); + doAnswer(new Answer<Void>() { + + @Override + public Void answer(InvocationOnMock invocation) throws Throwable { + successHandler.getValue().run(); + return null; + } + }).when(impl).execute(eq(vmInfoDao), eq(agentInfoDao), any(AgentId.class), any(VmId.class), eq(queue), + successHandler.capture(), any(Runnable.class)); + + DumpHeapCommand command = new DumpHeapCommand(impl); + + command.setVmInfoDAO(vmInfoDao); + command.setAgentInfoDAO(agentInfoDao); + command.setHeapDAO(heapDao); + command.setRequestQueue(queue); + + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addArgument(VmArgument.ARGUMENT_NAME, "foo"); + + command.run(factory.createContext(args)); + + verify(impl).execute(eq(vmInfoDao), eq(agentInfoDao), isA(AgentId.class), isA(VmId.class), eq(queue), + any(Runnable.class), any(Runnable.class)); + assertThat(factory.getOutput(), is(equalTo("Heap dump completed\n"))); + } + + @Test public void verifyNeedsVmId() throws CommandException { VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class);