possible data corruption while scrolling through rows

This is a multi-part message in MIME format.
--------------040908050805060203090304
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

hi,

I noticed the following bug in Rekall (I'm using version 2.2.3):

If a form block has a sub block, it can happen that the contents of some 
rows of the sub block is deleted, when the user presses for example the 
up or down arrow quickly two or more times in the parent block. (The 
problem may perhaps also occur for a simple form block, if row updates 
take a long time -- I didn't read the sources carefully enough to tell 
for sure...)

At some time during a call of KBFormBlock::doOperation e.g. for a down 
arrow keystroke, a progress bar is opened, and TKProgress::setDone calls 
qApp->processEvents. If the next key stroke arrives in the event queue 
before this call, processEvents causes another call of 
KBFormBlock::doOperation. The problem now is that 
KBFormBlock::doOperation is not reentrant safe. It happened to me quite 
often, that the first call of KBFormBlock::doOperation was at the point, 
where KBControl::clearValue() has been called, but where 
KBControl::getIniValue returns a non-empty value. Hence, during the 
second call of doOperation, KBControl::changed returns true, and all 
values of the affected row are deleted.

I fixed this (more or less...) by not allowing recursive calls of 
doOperation. See the attached patch. It detects recursive calls and 
queues them so that the "real execution" can be deferred until the first 
call of doOperation is finished. This may lead to an unexpected 
behaviour, because the execution sequence for events can be changed, if 
events are queued for different block at the same time. Any comments?

Abel

--------------040908050805060203090304
Content-Type: text/plain;
 name="kb_formblock.h.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="kb_formblock.h.diff"

--- rekall-2.2.3-orig/libs/kbase/kb_formblock.h	2004-11-18 19:26:23.000000000 +0100
+++ rekall-2.2.3/libs/kbase/kb_formblock.h	2005-01-17 00:14:49.000000000 +0100
@@ -24,6 +24,14 @@
 /*  -----------								*/
 /*  This class implements the block object when embedded in a form.	*/
 
+typedef struct	_opList
+{
+	KB::Action	action;
+	uint		toQRow;
+	KBTabOrderList	*tabList;
+	_opList		*next;
+} _opList;
+
 class LIBKBASE_API	KBFormBlock : public KBBlock, public KBNavigator
 {
 	Q_OBJECT
@@ -47,6 +55,9 @@
 
 	void			focusMovesRow	(uint)			;
 	void			focusMovesItem	(KBItem *, QFocusEvent::Reason) ;
+	bool			_doOperation	(KB::Action, uint, KBTabOrderList * = 0) ;
+	_opList			*queueFirst, *queueLast			;
+	int			inOperation				;
 
 public	:
 

--------------040908050805060203090304
Content-Type: text/plain;
 name="kb_formblock.cpp.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="kb_formblock.cpp.diff"

--- rekall-2.2.3-orig/libs/kbase/kb_formblock.cpp	2004-11-18 19:26:23.000000000 +0100
+++ rekall-2.2.3/libs/kbase/kb_formblock.cpp	2005-01-16 23:26:43.000000000 +0100
@@ -60,6 +60,9 @@
 	m_curItem  = 0	     ;
 	m_inQuery  = false   ;
 	m_dChanged = false   ;
+	queueFirst = queueLast = 0;
+	inOperation = 0	     ;
+	
 
 	if ((dx.getIntValue() == 0) && (dy.getIntValue() == 0))
 			dy.setValue (25) ;
@@ -92,6 +95,8 @@
 	m_curItem  = 0	     ;
 	m_inQuery  = false   ;
 	m_dChanged = false   ;
+	queueFirst = queueLast = 0;
+	inOperation = 0	     ;
 }
 
 /*  KBFormBlock								*/
@@ -115,6 +120,8 @@
 	m_curItem  = 0	     ;
 	m_inQuery  = false   ;
 	m_dChanged = false   ;
+	queueFirst = queueLast = 0;
+	inOperation = 0;     ;
 }
 
 /*  KBFormBlock								*/

--------------040908050805060203090304
Content-Type: text/plain;
 name="kb_blockact.cpp.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="kb_blockact.cpp.diff"

--- rekall-2.2.3-orig/libs/kbase/kb_blockact.cpp	2004-11-18 19:26:23.000000000 +0100
+++ rekall-2.2.3/libs/kbase/kb_blockact.cpp	2005-01-17 00:16:32.000000000 +0100
@@ -120,9 +120,49 @@
 		KBTabOrderList	*tabList
 	)
 {
+	if (inOperation)
+	{
+		_opList *p = (_opList*) malloc(sizeof(_opList));
+		if (!p) 
+			return false;
+		
+		if (!queueFirst)
+			queueFirst = p;
+		else
+			queueLast->next = p;
+		
+		p->action = action;
+		p->toQRow = toQRow;
+		p->tabList = tabList;
+		p->next = 0;
+		queueLast = p;
+		
+		return true; // xxx problematisch...
+	}
+	
+	inOperation++;
+	bool res = _doOperation(action, toQRow, tabList);
+	
+	while (queueFirst)
+	{
+		_opList *p = queueFirst;
+		_doOperation(p->action, p->toQRow, p->tabList);
+		queueFirst = p->next;
+		free(p);
+	}
+	inOperation--;
+	return res;
+}
+
+bool	KBFormBlock::_doOperation
+	(	KB::Action	action,
+		uint		toQRow,
+		KBTabOrderList	*tabList
+	)
+{
 	if (action == KB::Reload)
-		return	doOperation (KB::Query,   0, tabList) &&
-			doOperation (KB::Execute, 0, tabList) ;
+		return	_doOperation (KB::Query,   0, tabList) &&
+			_doOperation (KB::Execute, 0, tabList) ;
 	/* If the block is a null (menu-only) block then there is	*/
 	/* nothing to do within this block, but we do scan down through	*/
 	/* any nested form blocks or framers.				*/

--------------040908050805060203090304
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Rekall-devel mailing list
[email protected]
http://www.mailman.a-i-s.co.uk/cgi-bin/mailman/listinfo/rekall-devel
--------------040908050805060203090304--