Page MenuHomeFreeBSD

Make RFB_ENCODING_ZLIB message handling idempotent.
ClosedPublic

Authored by markj on Jun 17 2019, 3:33 PM.
Tags
None
Referenced Files
F103580207: D20673.diff
Tue, Nov 26, 5:58 PM
Unknown Object (File)
Sat, Nov 23, 6:34 AM
Unknown Object (File)
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)
Oct 24 2024, 6:42 AM
Unknown Object (File)
Oct 15 2024, 10:57 PM
Unknown Object (File)
Oct 6 2024, 5:33 PM

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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 24898
Build 23628: arc lint + arc unit

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.