Community
Participate
Working Groups
Created attachment 113601 [details] Proposed fix After thinking some more about how we deal with contexts as part of Bug 244749, the relationship between IContainerDMContext and IProcessDMContext, and the one between IExecutionDMContext and IThreadDMContext was bothering me. The context hierarchy is not simple and requires a bit of juggling is some places to get to the context. This bug is a proposal for a new hierarchy. The old hierarchy was: // MIControlDMContext // | // MIProcessDMC (IProcess) // MIContainerDMC _____/ | // (IContainer) | // | MIThreadDMC (IThread) // MIExecutionDMC _____/ // (IExecution) How about if we did this instead: // Process not being debugged Process being debugged // -------------------------- ---------------------- // // MIControlDMContext MIControlDMContext // | | // MIProcessDMC (IProcess) MIContainerDMC (IProcess and IContainer) // | | // MIThreadDMC (IThread) MIExecutionDMC (IThread and IExecution) The hierarchy is linear, creating context becomes simpler, with an MIContainerDMC one can easily use both IProcess or IContainer, and the same for MIExecutionDMC. Further, we currently use the same id for the OSThread as for the ExecutionThread, and the same id for the OSProcess as for the Container; so it makes this joint context quite simple. If the ids were different, it would not be a problem as we would simply store two ids in the class. Also, I'm hoping it may open the door to having more specific contexts use in IProcesses, although I'm not sure about that yet. The attached patch contains the necessary changes which seems to work well. The majority of changes are simplifications in the creation of the contexts. I would appreciate opinions. Thanks.
Yes, the complex hierarchy is pretty difficult to manage. Which makes me wonder if it's even worth it to support it in the IDMContext interface. The one potential breaking problem with a single hierarchy solution is that a process context which is not being debugged (which would be used in a process list view for example), should be equal to a process which is being debugged. So I think the simplified solution should be to have the IProcess and IThread interfaces always implemented by the same context classes, which also implement IContainer and IExecution (DMContexts). I.e. the hierarchies for processes being debugged and not being debugged should be the same.
(In reply to comment #1) > Yes, the complex hierarchy is pretty difficult to manage. Which makes me > wonder if it's even worth it to support it in the IDMContext interface. [...] > I.e. the hierarchies for processes > being debugged and not being debugged should be the same. So ultimately, IContainer and IProcess would be different names for the same thing (same for IThread and IExecution); and this would allow for the use of a specialized context within IProcesses and IRunControl without using each others context. So, you suggest that I get rid of MIProcessDMC and MIThreadDMC and always use MIContainerDMC and MIExecutionDMC. That is even easier than my suggestion. I do have a concern that if the GDB threadGroupId is no longer the pid, we would have to maintain two ids in the MIContainerDMC, which would imply that the context with the debugger attached would not be the same as with the debugger not attached. I'm not convinced that GDB will always use the pid...
(In reply to comment #2) > I do have a concern that if the GDB threadGroupId is no longer the pid, we > would have to maintain two ids in the MIContainerDMC, which would imply that > the context with the debugger attached would not be the same as with the > debugger not attached. I'm not convinced that GDB will always use the pid... This is definitely a big risk :-( Another alternative is what I had in the original IProcesses service interface: conversion method for debug and non-debug contexts IProcessDMContext getProcessForDebugContext(IDMContext dmc); IThreadDMContext getThreadForDebugContext(IDMContext dmc); (the reverse mappings were obtainable through DM data) This interface basically allowed the two context hierarchies to be independent, but it also complicated the UI quite a bit. We could go back to this and still use a single hierarchy, but we would have the option of changing that in the future if needed.
In reply to comment #3) > IProcessDMContext getProcessForDebugContext(IDMContext dmc); > IThreadDMContext getThreadForDebugContext(IDMContext dmc); > (the reverse mappings were obtainable through DM data) > > This interface basically allowed the two context hierarchies to be independent, > but it also complicated the UI quite a bit. > We could go back to this and still > use a single hierarchy, but we would have the option of changing that in the > future if needed. I wonder if this risk of complexity is worth making the change... Let's hash it out at the meeting. Another idea would be to take your solution of a single hierarchy of comment #1 "the hierarchies for processes being debugged and not being debugged should be the same", but to ignore the threadGroupId in the equals() method. Two context would be equal if the pid was the same; so if a process did not have the debugger attached it would have groupId of null, while if it was attached, it would have a real groupId, but the equal() would ignore that. I wonder if that is a solution or a hack? :-)
In the past I've been using "model" to refer to data model related bugs.
After our weekly meeting we decided that since the current solution works, and the new proposal is not as clean-cut as we had hoped, we would leave things as is.