From 8934f52ae631c7f7bb2681d67361f6b5cfbcc1c1 Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Mon, 5 Mar 2018 11:08:29 +0000 Subject: [PATCH] Bug 532035: Enable synchronizer to resynchronize/flushCaches Change-Id: Ib1ebbe5a1b87e9402d961383fcf15dae865ac0c5 --- .../mi/service/MIBreakpointsSynchronizer.java | 118 +++++++++++++++++- .../gdb/tests/GDBConsoleBreakpointsTest.java | 47 ++++++- 2 files changed, 161 insertions(+), 4 deletions(-) diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsSynchronizer.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsSynchronizer.java index c232b615c67..f2bacbcb11e 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsSynchronizer.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsSynchronizer.java @@ -19,16 +19,21 @@ package org.eclipse.cdt.dsf.mi.service; import java.io.File; import java.math.BigInteger; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; -import java.util.Queue; import java.util.Set; +import java.util.stream.Collector; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.cdt.core.IAddress; import org.eclipse.cdt.core.model.ITranslationUnit; @@ -48,8 +53,10 @@ import org.eclipse.cdt.dsf.concurrent.ImmediateRequestMonitor; import org.eclipse.cdt.dsf.concurrent.RequestMonitor; import org.eclipse.cdt.dsf.datamodel.DMContexts; import org.eclipse.cdt.dsf.datamodel.IDMContext; +import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointDMContext; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointDMData; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsTargetDMContext; +import org.eclipse.cdt.dsf.debug.service.ICachingService; import org.eclipse.cdt.dsf.debug.service.IDsfBreakpointExtension; import org.eclipse.cdt.dsf.debug.service.IProcesses.IProcessDMContext; import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext; @@ -65,6 +72,7 @@ import org.eclipse.cdt.dsf.gdb.internal.tracepointactions.TracepointActionManage import org.eclipse.cdt.dsf.gdb.internal.tracepointactions.WhileSteppingAction; import org.eclipse.cdt.dsf.mi.service.MIBreakpoints.MIBreakpointDMContext; import org.eclipse.cdt.dsf.mi.service.MIBreakpointsManager.IMIBreakpointsTrackingListener; +import org.eclipse.cdt.dsf.mi.service.command.output.MIBreakListInfo; import org.eclipse.cdt.dsf.mi.service.command.output.MIBreakpoint; import org.eclipse.cdt.dsf.service.AbstractDsfService; import org.eclipse.cdt.dsf.service.DsfServiceEventHandler; @@ -101,7 +109,7 @@ import org.osgi.framework.BundleContext; * * @since 4.2 */ -public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMIBreakpointsTrackingListener { +public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMIBreakpointsTrackingListener, ICachingService { // Catchpoint expressions private static final String CE_EXCEPTION_CATCH = "exception catch"; //$NON-NLS-1$ @@ -148,6 +156,11 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI MIBreakpoint created; MIBreakpoint modified; String deleted; + static class BreakpointEventSynchronize { + IBreakpointsTargetDMContext dmc; + MIBreakListInfo list; + } + BreakpointEventSynchronize synchronize; } /** @@ -155,7 +168,7 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI * * @see MIBreakpointsSynchronizer class documentation for design comments */ - private Queue fBreakpointEvents = new LinkedList<>(); + private Deque fBreakpointEvents = new LinkedList<>(); /** * True if the delayed events processing task is idle. If idle, a new event @@ -297,11 +310,109 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI doTargetBreakpointDeleted(event.deleted, rm); } else if (event.modified != null) { doTargetBreakpointModified(event.modified, rm); + } else if (event.synchronize != null) { + doTargetBreakpointsSynchronized(event.synchronize.dmc, event.synchronize.list, rm); } else { rm.done(); } } + /** + * The effect of flushing the cache of the synchronizer is to recollect all + * breakpoint info from GDB and update the IBreakpoints and MIBreakpointManager + * services too. + * + * Note that an optimization in the number of calls to synchronize can be done, see + * synchronize's removeBpsForAllDmcs parameter. + */ + @Override + public void flushCache(IDMContext context) { + Collection contexts; + IBreakpointsTargetDMContext breakpointsTargetDMContext = DMContexts.getAncestorOfType(context, + IBreakpointsTargetDMContext.class); + if (breakpointsTargetDMContext != null) { + contexts = Arrays.asList(breakpointsTargetDMContext); + } else { + contexts = getBreakpointsManager().getTrackedBreakpointTargetContexts(); + } + + for (IBreakpointsTargetDMContext bpContext : contexts) { + synchronize(bpContext, false); + } + } + + /** + * Synchronize the breakpoint state with the back end. This is done by issuing a + * -break-list to the backend and adding that result to the list of queue'd + * events from the backend. When this entry in the queue is processed, it + * converts itself to a series of new events that represent the difference + * between the state in the breakpoint manager and GDB. + * + * @param bpContext + * context to issue MI Break List on + * @param removeBpsForAllDmcs + * If the break list command returns breakpoints for all contexts, + * pass true. If false, the synchronizer assumes only bps not listed + * for bpContext will be removed. This provides an optimization to + * prevent issuing createMIBreakList multiple times that will return + * the same value. (Note that using a CommandCache will not achieve + * the optimization because each call to createMIBreakList is a + * different context.) + * @since 5.5 + */ + protected void synchronize(IBreakpointsTargetDMContext bpContext, boolean removeBpsForAllDmcs) { + fConnection.queueCommand(fConnection.getCommandFactory().createMIBreakList(bpContext), + new DataRequestMonitor(getExecutor(), null) { + @Override + protected void handleSuccess() { + BreakpointEvent event = new BreakpointEvent(); + event.synchronize = new BreakpointEvent.BreakpointEventSynchronize(); + event.synchronize.dmc = removeBpsForAllDmcs ? null : bpContext; + event.synchronize.list = getData(); + queueEvent(event); + } + }); + } + + private void doTargetBreakpointsSynchronized(IBreakpointsTargetDMContext breakpointsContext, MIBreakListInfo data, + RequestMonitor rm) { + final MIBreakpointsManager bm = getBreakpointsManager(); + Map> bpToPlatformMaps = bm + .getBPToPlatformMaps(); + Stream breakpointsKnownToManager = bpToPlatformMaps.entrySet().stream() + .flatMap(m -> m.getValue().keySet().stream()); + Collector> collector = Collectors.toMap( + (MIBreakpointDMContext dmc) -> DMContexts.getAncestorOfType(dmc, IBreakpointsTargetDMContext.class), + (MIBreakpointDMContext dmc) -> dmc.getReference()); + Map numbersKnownToManager = breakpointsKnownToManager + .filter(MIBreakpointDMContext.class::isInstance).map(MIBreakpointDMContext.class::cast) + .collect(collector); + + for (MIBreakpoint miBpt : data.getMIBreakpoints()) { + String number = miBpt.getNumber(); + if (numbersKnownToManager.values().remove(number)) { + BreakpointEvent event = new BreakpointEvent(); + event.modified = miBpt; + fBreakpointEvents.addFirst(event); + } else { + BreakpointEvent event = new BreakpointEvent(); + event.created = miBpt; + fBreakpointEvents.addFirst(event); + } + } + for (Entry entry : numbersKnownToManager.entrySet()) { + IBreakpointsTargetDMContext dmc = entry.getKey(); + String number = entry.getValue(); + if (number != null && !number.isEmpty() && (breakpointsContext == null || breakpointsContext.equals(dmc))) { + BreakpointEvent event = new BreakpointEvent(); + event.deleted = number; + fBreakpointEvents.addFirst(event); + } + } + + rm.done(); + } + public void targetBreakpointCreated(final MIBreakpoint miBpt) { BreakpointEvent event = new BreakpointEvent(); event.created = miBpt; @@ -1869,4 +1980,5 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI (CE_EXCEPTION_CATCH.equals(miBpt.getExpression()) || CE_EXCEPTION_THROW.equals(miBpt.getExpression())))); } + } diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBConsoleBreakpointsTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBConsoleBreakpointsTest.java index 785a7a72ded..9807f28e5b5 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBConsoleBreakpointsTest.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBConsoleBreakpointsTest.java @@ -35,13 +35,16 @@ import org.eclipse.cdt.debug.internal.core.breakpoints.CFunctionBreakpoint; import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor; import org.eclipse.cdt.dsf.concurrent.Query; import org.eclipse.cdt.dsf.datamodel.DMContexts; +import org.eclipse.cdt.dsf.debug.service.IBreakpoints; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsAddedEvent; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsChangedEvent; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsRemovedEvent; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsTargetDMContext; import org.eclipse.cdt.dsf.debug.service.IBreakpoints.IBreakpointsUpdatedEvent; import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext; +import org.eclipse.cdt.dsf.debug.service.command.IEventListener; import org.eclipse.cdt.dsf.gdb.service.command.IGDBControl; +import org.eclipse.cdt.dsf.mi.service.MIBreakpointsSynchronizer; import org.eclipse.cdt.dsf.mi.service.command.output.MIBreakListInfo; import org.eclipse.cdt.dsf.mi.service.command.output.MIBreakpoint; import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo; @@ -104,6 +107,8 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { private DsfServicesTracker fServicesTracker; protected IBreakpointsTargetDMContext fBreakpointsDmc; private IGDBControl fCommandControl; + private IBreakpoints fBreakpointService; + private MIBreakpointsSynchronizer fBreakpointsSynchronizer; private List fBreakpointEvents = new ArrayList(); @@ -123,6 +128,12 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { fCommandControl = fServicesTracker.getService(IGDBControl.class); Assert.assertTrue(fCommandControl != null); + fBreakpointService = fServicesTracker.getService(IBreakpoints.class); + Assert.assertTrue(fBreakpointService != null); + + fBreakpointsSynchronizer = fServicesTracker.getService(MIBreakpointsSynchronizer.class); + Assert.assertTrue(fBreakpointsSynchronizer != null); + // Register to breakpoint events fSession.addServiceEventListener(GDBConsoleBreakpointsTest.this, null); } @@ -488,7 +499,32 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { } } - private void testConsoleBreakpoint(Class type, Map attributes) throws Throwable { + /** + * Run a set of console breakpoint tests, twice. Once using events from GDB, and + * then again with manual refreshes to make sure we get the same results. + */ + private void testConsoleBreakpoint(Class type, Map attributes) + throws Throwable { + testConsoleBreakpointStandard(type, attributes, () -> { + }); + fBreakpointEvents.clear(); + + /* + * Run the test without the breakpoints service handling the updates via async + * messages to ensure we end up with the same behaviour from refreshing + * manually. Because we want to test the manual refreshing behaviour, we need to + * stop listening to those async messages to do that we temporarily remove the + * breakpoint service from the event listeners + */ + fCommandControl.removeEventListener((IEventListener) fBreakpointService); + try { + testConsoleBreakpointStandard(type, attributes, () -> fBreakpointsSynchronizer.flushCache(null)); + } finally { + fCommandControl.addEventListener((IEventListener) fBreakpointService); + } + } + + private void testConsoleBreakpointStandard(Class type, Map attributes, Runnable flushCache) throws Throwable { // Set a console breakpoint and verify that // the corresponding platform breakpoint is created // and its install count is 1 if the breakpoint is installed @@ -497,6 +533,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { setConsoleBreakpoint(type, attributes); MIBreakpoint[] miBpts = getTargetBreakpoints(); Assert.assertTrue(miBpts.length == 1); + flushCache.run(); waitForBreakpointEvent(IBreakpointsAddedEvent.class); Assert.assertTrue(getPlatformBreakpointCount() == 1); ICBreakpoint plBpt = findPlatformBreakpoint(type, attributes); @@ -518,12 +555,14 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { // Disable the console breakpoint and verify that // the platform breakpoint is disabled. enableConsoleBreakpoint(miBpts[0].getNumber(), false); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(!plBpt.isEnabled()); // Enable the console breakpoint and verify that // the platform breakpoint is enabled. enableConsoleBreakpoint(miBpts[0].getNumber(), true); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(plBpt.isEnabled()); @@ -531,6 +570,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { // verify that the platform breakpoint's ignore count // is updated. setConsoleBreakpointIgnoreCount(miBpts[0].getNumber(), 5); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(plBpt.getIgnoreCount() == 5); @@ -538,6 +578,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { // verify that the platform breakpoint's ignore count // is updated. setConsoleBreakpointIgnoreCount(miBpts[0].getNumber(), 0); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(plBpt.getIgnoreCount() == 0); @@ -545,6 +586,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { // verify that the platform breakpoint's condition // is updated. setConsoleBreakpointCondition(miBpts[0].getNumber(), "path==0"); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(plBpt.getCondition().equals("path==0")); @@ -552,12 +594,14 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { // verify that the platform breakpoint's condition // is updated. setConsoleBreakpointCondition(miBpts[0].getNumber(), ""); + flushCache.run(); waitForBreakpointEvent(IBreakpointsUpdatedEvent.class); Assert.assertTrue(plBpt.getCondition().isEmpty()); // Delete the console breakpoint and verify that // the install count of the platform breakpoint is 0. deleteConsoleBreakpoint(miBpts[0].getNumber()); + flushCache.run(); waitForBreakpointEvent(IBreakpointsRemovedEvent.class); Assert.assertTrue(getPlatformBreakpointCount() == 1); plBpt = findPlatformBreakpoint(type, attributes); @@ -578,6 +622,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase { setConsoleBreakpoint(type, attributes); miBpts = getTargetBreakpoints(); Assert.assertTrue(miBpts.length == 1); + flushCache.run(); waitForBreakpointEvent(IBreakpointsAddedEvent.class); Assert.assertTrue(getPlatformBreakpointCount() == 1);