[THRIFT-2680] c_glib: ThriftFramedTransport fails when peer unexpectedly closes connection

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 0.9.1
• Fix Version/s: 0.9.2
• Component/s: C glib - Library
• Labels: c_glib library

Environment

Fedora 20 64-bit on x86_64

• Patch Info: Patch Available
• Assignee: Roger Meier
• Reporter: Simon South
• Created: 28/Aug/14 03:07
• Updated: 05/Nov/14 04:49
• Resolved: 01/Sep/14 18:22

Description

Working on the integration test suite for c_glib exposed a problem with ThriftFramedTransport: If the peer closes the connection unexpectedly, thrift_framed_transport_read_frame will fail with either a critical GLib warning or a segmentation fault.

The attached patches fix this issue. The changes are, in order:

1. Add a suitable test case to testframedtransport.c.

2. Modify thrift_framed_transport_read_frame and thrift_framed_transport_read_frame_slow so they check for and respond to errors returned from other Thrift methods. Additionally, thrift_framed_transport_read_frame_slow should now always return -1 in case of read error, which includes "successfully" reading zero bytes from the peer (which generally indicates a prematurely closed connection).

I've also taken the liberty of gently rewriting thrift_framed_transport_read_frame so it

• Correctly uses an unsigned integer variable to hold the frame's size,

• Uses thrift_transport_... functions to invoke class methods instead of the rather brittle practice of dereferencing class members directly, and

• Accurately reports success or failure.

Issue Links

• is required by: THRIFT-2685 c_glib: Include in integration test suite