Community
Participate
Working Groups
The ThreadsLayoutNode is meant to be the generic layout node based strictly on APIs in on org.eclipse.dd.dsf.debug. But since the MI implementation is hard-coded to return a single thread, the ThreadsLayoutNode is stuck with a dependency on org.eclipse.dd.dsf.mi. Along with making this move, the LaunchViewModelProvider can be extended to configure its layout.
ThreadsVMNode and ContainerVMNode are currently in GDB, there is a lot of code in them that is common to all debuggers, but there are parts of it that are necessarily debugger specific. The best we ccan do is create abstract base classes of these two nodes and add them to org.eclipse.dd.dsf.debug.ui plugin, then extend these base classes in the GDB and PDE implementations.
I'll try to come up with a patch.
Thanks Toni, Let's target this post 1.0. We are going to branch 1.0 after RC1 so that we can continue active development in head.
Created attachment 101016 [details] Refactoring of ThreadVMNode This patch extracts the abstract base class AbstractThreadVMNode from ThreadVMNode. I also looked at ContainerVMNode, but I don't see much code that can be shared, although there are some potential refactorings that could make sense, eg. - Create interfaces for IContainer(Started|Exited)Event in IRunControl - Let GDB's Inferior(Started|Exited)Event implement those interfaces - Implement IRunControl.getExecutionData() in GDBRunControl such that it can be used as replacement for GDBRunControl.getThreadData() and GDBRunControl.getProcessData() I think, this would help factoring out a common abstract imlementation of ContainerVMNode.
Thank you Toni, The patch is exactly what I had in mind. I only have a couple of comments: 1) I think we should leave out the implementation of IElementMementoProvider from the base class for now. There is a separate bug (bug 231651), to abstract out this implementation into a lower-level base class, and we need to hash out what the protected method signature is going to be (async or sync). 2) The container could also be implemented using the IStated/IExitedDMEvent, because inferior process events do implement these as well. The event handling could distinguish these by checking whether the started process is a container or not. The other issue here is that there is no generic interface for GDBStartedEvent. This event is handled in case of an attach to process, where an inferior process started event may not be generated. I'm not sure if we really need a generic version of this event, for now we could just handle it in the GDB sub-class of the container vm node.
Created attachment 101222 [details] Reworked patch (+committed) Thanks, Pawel. I have updated the patch with the suggested changes: - moved implementation of IElementMementoProvider back to ThreadVMNode - factored out AbstractContainerVMNode and implemented event handling as generic as possible
Thanks Toni, I committed the updated patch (after merging in bug 233339). Randy please review.
Pawel The changes look OK except for the following ContainerVMNode's getDeltaFlags/buildDelta were handling the events InferiorExitedDMEvent * InferiorStartedDMEvent AbstractContainerVMNode is not handling them and they do not appear to be in ContainerVMNode anymore. Unless my diff is not showing me something. I will update from HEAD to be sure. Randy
(In reply to comment #8) > Pawel > > The changes look OK except for the following > > ContainerVMNode's getDeltaFlags/buildDelta were handling the events > > InferiorExitedDMEvent * InferiorStartedDMEvent > > AbstractContainerVMNode is not handling them and they do not appear > to be in ContainerVMNode anymore. Unless my diff is not showing me > something. > > I will update from HEAD to be sure. > > Randy > Hi Randy, the container events are handled in the base class generically. The container events are distinguished from the thread events by looking at the execution context of the event. See also comment 5 pt.2.
(In reply to comment #9) Agreed. Randy, does this answer your question?
DD 1.1 reelased!
