Page MenuHomeFreeBSD

Make RFB_ENCODING_ZLIB message handling idempotent.
ClosedPublic

Authored by markj on Jun 17 2019, 3:33 PM.
Tags
None
Referenced Files
F81655251: D20673.diff
Fri, Apr 19, 1:07 PM
F81633630: D20673.diff
Fri, Apr 19, 7:01 AM
Unknown Object (File)
Mon, Apr 15, 11:59 PM
Unknown Object (File)
Thu, Apr 11, 8:00 AM
Unknown Object (File)
Jan 24 2024, 8:02 AM
Unknown Object (File)
Dec 22 2023, 8:59 PM
Unknown Object (File)
Dec 22 2023, 1:30 AM
Unknown Object (File)
Dec 8 2023, 12:23 AM

Details

Summary

The VNC server uses two threads to manage a connection: one thread sends
periodic updates to the client, and another handles client requests.
For the most part they are unsynchonized, and we may be simultaneously
handling a request and pushing an update.

Fix two bugs in the RFB_ENCODING_ZLIB message handler:

  • Ensure that we don't reinitialize the zlib stream if multiple messages are received.
  • Don't indicate to the sending thread that zlib compression is to be used until we've finished initializing the stream.

This is not a complete fix but I wanted to have something small that can
easily be MFCed. In particular, it is insufficient on platforms where
stores can be reordered (which I guess is relevant since there is an
arm64 port).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

As a first-cut workaround, LGTM.

Longer term, I don't like that there's no real synchronization between the two threads. Also I think this could pretty easily be converted to a single thread, obviating the need for synchronization.

This revision is now accepted and ready to land.Jun 17 2019, 3:39 PM
In D20673#446746, @cem wrote:

As a first-cut workaround, LGTM.

Longer term, I don't like that there's no real synchronization between the two threads. Also I think this could pretty easily be converted to a single thread, obviating the need for synchronization.

Agreed. I'll try to find out why it was written this way.

In agreement on it probably needs cleaned up, but first finding out why it was done this way as a safe position to take.

This revision was automatically updated to reflect the committed changes.