Page MenuHomeFreeBSD

Make RFB_ENCODING_ZLIB message handling idempotent.
ClosedPublic

Authored by markj on Jun 17 2019, 3:33 PM.
Tags
None
Referenced Files
F103230346: D20673.diff
Fri, Nov 22, 11:07 AM
Unknown Object (File)
Sat, Nov 16, 7:37 PM
Unknown Object (File)
Fri, Nov 15, 8:36 AM
Unknown Object (File)
Thu, Oct 24, 6:42 AM
Unknown Object (File)
Oct 15 2024, 10:57 PM
Unknown Object (File)
Oct 6 2024, 5:33 PM
Unknown Object (File)
Oct 3 2024, 2:24 AM
Unknown Object (File)
Oct 2 2024, 8:09 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.