Bug 3190

Summary: Buggy StatusWidget
Product: frysk Reporter: Sami Wagiaalla <swagiaal>
Component: generalAssignee: Sami Wagiaalla <swagiaal>
Status: NEW ---    
Severity: normal CC: pmuldoon
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Bug Depends on:    
Bug Blocks: 1632    

Description Sami Wagiaalla 2006-09-09 15:11:40 UTC
The problems are:

First of all a GuiTask does not reflect the observers that a GuiProc has (and if
the GuiProc has a syscall observer, that means it is a cascading observer, and
all tasks should have it). All of our stuff works on GuiTasks and GuiProcs so
this stuff is braindead :(

Because a GuiTask does not know what observers it has, the only thing plotted to
the timeline is the first initial line (ie the top line, the thread lines stay
forever blank).

also, If you have say thunderbird in the eventviewer, and bash forks. Then the
event is drawn to all timelines.
Comment 1 Phil Muldoon 2006-09-09 16:08:41 UTC
It's not the status widget that is buggy, but the whole cascading relationship
of GuiProc -> GuiTask. As we know when we add a cascading observer to a GuiProc,
it is just an illusion. All those observers are really tasks observers. 

Until recently there was no formal relationship between a GuiProc and a GuiTask!
I fixed that several weeks ago when GuiTasks were added. The next problem is to
append the actual observer on the GuiTasks involved, when a cascading observer
is applied to the a GuiProc.

The second issues is that GenericActions{Points} seems broken. When I add say a
Syscall observer to Foo and Bar and add a generic action for both, the execute()
method does not have a Task parameter in the generic action. When a syscall
"fires" on Bar, for some reason, the action point on Foo is being called too
even though no syscall happened in Task Foo.

I fixed this is StatusWidget by changing TimelineAction from a GenericAction to
a TaskActionPoint, and filtering on the PID in the execute function. This
requires per instance filtering as well, which is clumsy.  There is still a lot
of work to be done in both areas.

Changelog entries reflecting what I have changed:

	2006-09-08  Phil Muldoon  <pmuldoon@redhat.com>
	
	* StatusWidget.java (TimelineAction.createEvent): Only add
	one glyph per observer type.
	(addObserverActionPoint): Changed clonedTaskActionPoint to
	cloningTaskActionPoint.
	(addObserverActionPoint): Changed forkingTaskActionPoint to
	forkedTaskActionPoint.

	2006-09-08  Phil Muldoon  <pmuldoon@redhat.com>
	
	* StatusWidget.java (addObserverActionPoint): New.
	(initEventViewer): Don't use generic action points (broken).
	(TimelineAction): Rebase on TaskActionPoint, not Generic.
	(TimelineAction.execute): Filter on passed task.

2006-09-07  Phil Muldoon  <pmuldoon@redhat.com>
	
	* observers/TaskSyscallObserver.java (runEnterActions): Remove double action call
	(runExitActions): Ditto.
	
	* StatusWidget.java (newTrace): Rename variable from trace1 to
	newTrace.
	(StatusWidget): Rename trace0 to initialTrace
	(buildEventViewer): New.
	(StatusWidget): Refactor viewer init to buildEventViewer.
	(initThreads): Rename thread linked list to threadList.
	(initLogTextView): Rename to initEventViewer.
Comment 2 Sami Wagiaalla 2006-09-10 19:10:35 UTC
I changed it so that TaskObserverRoot notifies the concerned GuiTask
whenever and observer is added to it. As a result the GuiTask is aware
of observers added though Cascading Observer or those added directly to
the task.

A still open issue is when should the proc be told that an observer (Cascading
observer) has been added to it ? What does removing a Cascading Observer mean ?

2006-09-10    <swagiaal@redhat.com>

	* observers/TaskObserverRoot.java: Added handling for addedTo and 
	removedFrom, which just forward the call to the GuiTask (part of fix
	for bz 3190).
	* observers/ObserverRoot.java: Removed onAdded and onRemoved runnables
	and all references.
	* observers/AttachedContinueObserver.java: Removed
	* observers/AttachedResumeObserver.java: Removed
	* observers/AttachedStopObserver.java: Removed
	* observers/DetachedContinueObserver.java: Removed
	* GuiTask.java: Removed observer.onAdded/onRemoved Runnables. This
	is now done by TaskObserverRoot.
	Added functions observerRemoved, observerAdded.
	* GuiProc.java: Removed observer.onAdded/onRemoved Runnables. This
	is now done by TaskObserverRoot.
	* EventLogger.java: Removed references to AttachedContinueObserver
	AttachedResumeObserver, AttachedStopObserver, 
	DetachedContinueObserver.
Comment 3 Phil Muldoon 2006-09-12 23:19:20 UTC
In continuing this fix it was found that DebugProcess when loaded from disk was
in fact creating a reference to the existing base observers instead of cloning
its own. This meant that one observer was recycled among all Tasks and was
causing part of the problem in StatusWidget. This also required a reference fix
the druid:

2006-09-12  Phil Muldoon  <pmuldoon@localhost.localdomain>
	
	* druid/CreateFryskSessionDruid.java (getProcessObserverControls): Use
	setCheckedbyName, instead of setChecked().
	* sessions/DebugProcess.java (load): Create a copy of the observer
	and do not add a reference to the base observer.

Also, as a leftover from the time when the eventviewer only had one trace (one
per process, on per thread), the markerId was only tracking the most recent
marker added. Fixed this by building a hashmap of observer type, and add the
traceID as the lookup value:

	2006-09-12  Phil Muldoon  <pmuldoon@redhat.com>
	
	* StatusWidget.java (TimeLine): Build a Hashmap of marker's to
	x-ref on actual observer execution.

These two fixes, along with the others, now enable multiple event tracking and
plotting per thread in the eventviewer.