Bug 3722

Created attachment 3037 [details] [review]
A (dubious?) patch to fix the leak.

Created attachment 3038 [details] [review]
evdev-repeat-sanely-1.patch

Created attachment 3039 [details] [review]
evdev-xkb-2.patch

Created attachment 3040 [details]
a screenshot without patch

Created attachment 3041 [details]
a screenshot with patch

(In reply to comment #5)
> Created an attachment (id=3041) [edit]
> my fix of RECORD extension

this attachment is not 6.8.2 but XFree86-4.3.0.
6.8.2 has same problem.

Original reporter e-mail and comments lost in bugzilla disk death.   xorg-team
archives show:

           Summary: RECORD extension generates X protocol violation
           Product: xorg
           Version: 6.8.2
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Server/general
        AssignedTo: xorg-team at lists.x.org
        ReportedBy: karakama-akira at naka.hitachi-hitec.com


RECORD extension generates X protocol violation.
This is the bug of RECORD's reply buffer management algorithm.

RecordAProtocolElement (...)	/* xc/programs/Xserver/record/record.c */
{
		:
	if (futurelen >= 0)
	{ /* start of new protocol element */
		:
		if (!pContext->numBufBytes)	/* reply buffer is empty */
		{
			create reply packet header
		}
		:
		/* adjust reply length in reply buffer */
		:
	} /* end if not continued reply */

	/* if space available >= space needed, buffer the data */

	if (REPLY_BUF_SIZE - pContext->numBufBytes >= datalen + 
numElemHeaders)
	{
		:
	}
	else
		RecordFlushReplyBuffer ();
} /* RecordAProtocolElement */


The point is:

	o RECORD's reply packet header is created if
		o recording a new protocol element (not a continued one) and
		o reply buffer is empty


	o recording element (both new one and continued one) is buffered
	  if space available

consider this scenario;

	step1: RECORD's reply buffer is empty.

	step2: X server calls WriteToClient() to send a part of reply
		(names "A1")

	step3: RecordAProtocolElement is called.
		it creates reply packet header in RECORD's reply
		buffer because the buffer is empty.
		then buffer "A1" with protocol element header.

	step4: X server calls WriteToClient() to send continued part of the
		same reply (names "A2")

	step5: RecordAProtocolElement is called again.
		reply buffer space is not enough to store "A2".
		so flush reply buffer (i.e. "A1") and send "A2". ("A2" is not
		buffered). now reply buffer is empty.

	step6: X server calls WriteToClient() to send the rest of the same
		reply (names "A3")

	step7: RecordAProtocolElement is called again.
		buffers "A3" really top of the reply buffer.
		no packet header is created because this reply is continued 
one.
	step8: X server calls WriteToClient() to send NEW reply (names "B")

	step9: RecordAProtocolElement is called again.
		append "B" to reply buffer with protocol element because this
		is a new reply.
		but reply packet header is not created because reply buffer
		is not empty.

	......

So, recording client will receive protocol violated stream like below.

	[reply packet header]-[protocol element header]-[A1]-[A2]-[A3]-
	[protocol element header]-[B].

of course, expected stream is;

	[reply packet header]-[protocol element header]-[A1]-[A2]-[A3]-
	[reply packet header]-[protocol element header]-[B].

my solution is simple; not buffer continued reply if reply buffer is empty.    
     

A testcase and patch were also previously attached and since lost in bugzilla
disk death.

Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.

shame we lost the patch.  of course, record's reply semantics also seem quite broken, given the crashes i've been seeing internally with our endurance testing tool.  yay!

https://gitlab.freedesktop.org/xorg/xserver/issues/330