Skip to content

Commit a0e7de4

Browse files
AlexPeshkoffAlexPeshkoff
committed
Fixed CORE-6468: Wire compression causes sporadic "Error reading data from the connection." errors.
1 parent 17795e7 commit a0e7de4

5 files changed

Lines changed: 35 additions & 26 deletions

File tree

‎src/remote/inet.cpp‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ class Select
292292
#ifdef WIRE_COMPRESS_SUPPORT
293293
if (slct_zport)
294294
{
295-
if ((slct_zport->port_flags & PORT_z_data) &&
295+
if (slct_zport->port_z_data &&
296296
(slct_zport->port_state != rem_port::DISCONNECTED))
297297
{
298298
port = slct_zport;
@@ -319,7 +319,7 @@ class Select
319319
return SEL_NO_DATA;
320320

321321
#ifdef WIRE_COMPRESS_SUPPORT
322-
if (slct_port->port_flags & PORT_z_data)
322+
if (slct_port->port_z_data)
323323
return SEL_READY;
324324
#endif
325325

@@ -337,7 +337,7 @@ class Select
337337
HandleState ok(const rem_port* port)
338338
{
339339
#ifdef WIRE_COMPRESS_SUPPORT
340-
if (port->port_flags & PORT_z_data)
340+
if (port->port_z_data)
341341
return SEL_READY;
342342
#endif
343343
SOCKET n = port->port_handle;
@@ -2046,12 +2046,11 @@ static rem_port* receive( rem_port* main_port, PACKET * packet)
20462046
do {
20472047
if (!xdr_protocol(&main_port->port_receive, packet))
20482048
{
2049-
packet->p_operation = main_port->port_flags & PORT_partial_data ? op_partial : op_exit;
2050-
main_port->port_flags &= ~PORT_partial_data;
2051-
2052-
if (packet->p_operation == op_exit) {
2049+
packet->p_operation = main_port->port_partial_data ? op_partial : op_exit;
2050+
if (packet->p_operation == op_exit)
20532051
main_port->port_state = rem_port::BROKEN;
2054-
}
2052+
2053+
main_port->port_partial_data = false;
20552054
break;
20562055
}
20572056
#ifdef DEBUG
@@ -2118,7 +2117,7 @@ static bool select_multi(rem_port* main_port, UCHAR* buffer, SSHORT bufsize, SSH
21182117
*length = 0;
21192118
}
21202119
#ifdef WIRE_COMPRESS_SUPPORT
2121-
if (port->port_flags & PORT_z_data)
2120+
if (port->port_z_data)
21222121
INET_select->setZDataPort(port);
21232122
#endif
21242123
return (*length) ? true : false;
@@ -2146,7 +2145,7 @@ static bool select_multi(rem_port* main_port, UCHAR* buffer, SSHORT bufsize, SSH
21462145
*length = 0;
21472146
}
21482147
#ifdef WIRE_COMPRESS_SUPPORT
2149-
if (port->port_flags & PORT_z_data)
2148+
if (port->port_z_data)
21502149
INET_select->setZDataPort(port);
21512150
#endif
21522151
return (*length) ? true : false;
@@ -2806,7 +2805,7 @@ static bool inet_read( XDR* xdrs)
28062805
}
28072806

28082807
SSHORT length = end - p;
2809-
port->port_flags &= ~PORT_z_data;
2808+
port->port_z_data = false;
28102809
if (!REMOTE_inflate(port, packet_receive2, (UCHAR*)p, length, &length))
28112810
return false;
28122811
p += length;

‎src/remote/protocol.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,11 @@ bool_t xdr_protocol(XDR* xdrs, PACKET* p)
295295
return P_FALSE(xdrs, p);
296296

297297
#if COMPRESS_DEBUG > 1
298-
fprintf(stderr, "operation=%d %c\n", p->p_operation,
299-
xdrs->x_op == XDR_ENCODE ? 'E' : xdrs->x_op == XDR_DECODE ? 'D' : xdrs->x_op == XDR_FREE ? 'F' : 'U');
298+
if (xdrs->x_op != XDR_FREE)
299+
{
300+
fprintf(stderr, "operation=%d %c\n", p->p_operation,
301+
xdrs->x_op == XDR_ENCODE ? 'E' : xdrs->x_op == XDR_DECODE ? 'D' : xdrs->x_op == XDR_FREE ? 'F' : 'U');
302+
}
300303
#endif
301304

302305
switch (p->p_operation)

‎src/remote/remote.cpp‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -744,10 +744,13 @@ bool_t REMOTE_getbytes (XDR* xdrs, SCHAR* buff, unsigned bytecount)
744744
}
745745

746746
rem_port* port = (rem_port*) xdrs->x_public;
747-
Firebird::RefMutexGuard queGuard(*port->port_que_sync, FB_FUNCTION);
747+
Firebird::RefMutexEnsureUnlock queGuard(*port->port_que_sync, FB_FUNCTION);
748+
queGuard.enter();
748749
if (port->port_qoffset >= port->port_queue.getCount())
749750
{
750-
port->port_flags |= PORT_partial_data;
751+
queGuard.leave();
752+
753+
port->port_partial_data = true;
751754
return FALSE;
752755
}
753756

@@ -1405,7 +1408,7 @@ bool REMOTE_inflate(rem_port* port, PacketReceive* packet_receive, UCHAR* buffer
14051408
#ifdef COMPRESS_DEBUG
14061409
fprintf(stderr, "Inflate error\n");
14071410
#endif
1408-
port->port_flags &= ~PORT_z_data;
1411+
port->port_z_data = false;
14091412
return false;
14101413
}
14111414
#ifdef COMPRESS_DEBUG
@@ -1418,9 +1421,9 @@ bool REMOTE_inflate(rem_port* port, PacketReceive* packet_receive, UCHAR* buffer
14181421
if (strm.next_out != buffer)
14191422
break;
14201423

1421-
if (port->port_flags & PORT_z_data) // Was called from select_multi() but nothing decompressed
1424+
if (port->port_z_data) // Was called from select_multi() but nothing decompressed
14221425
{
1423-
port->port_flags &= ~PORT_z_data;
1426+
port->port_z_data = false;
14241427
return false;
14251428
}
14261429

@@ -1437,7 +1440,7 @@ bool REMOTE_inflate(rem_port* port, PacketReceive* packet_receive, UCHAR* buffer
14371440
SSHORT l = (SSHORT) (port->port_buff_size - strm.avail_in);
14381441
if ((!packet_receive(port, strm.next_in, l, &l)) || (l <= 0)) // fixit - 2 ways to report errors in same routine
14391442
{
1440-
port->port_flags &= ~PORT_z_data;
1443+
port->port_z_data = false;
14411444
return false;
14421445
}
14431446

@@ -1446,12 +1449,12 @@ bool REMOTE_inflate(rem_port* port, PacketReceive* packet_receive, UCHAR* buffer
14461449

14471450
*length = (SSHORT) (buffer_length - strm.avail_out);
14481451
if (strm.avail_in) // Z-buffer still has some data - probably can call inflate() once more on them
1449-
port->port_flags |= PORT_z_data;
1452+
port->port_z_data = true;
14501453
else
1451-
port->port_flags &= ~PORT_z_data;
1454+
port->port_z_data = false;
14521455

14531456
#ifdef COMPRESS_DEBUG
1454-
fprintf(stderr, "Z-buffer %s\n", port->port_flags & PORT_z_data ? "has data" : "is empty");
1457+
fprintf(stderr, "ZLib buffer %s\n", port->port_z_data ? "has data" : "is empty");
14551458
#endif
14561459

14571460
return true;

‎src/remote/remote.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -977,13 +977,13 @@ const USHORT PORT_async = 0x0002; // Port is asynchronous channel for events
977977
const USHORT PORT_no_oob = 0x0004; // Don't send out of band data
978978
const USHORT PORT_disconnect = 0x0008; // Disconnect is in progress
979979
const USHORT PORT_dummy_pckt_set= 0x0010; // A dummy packet interval is set
980-
const USHORT PORT_partial_data = 0x0020; // Physical packet doesn't contain all API packet
980+
//const USHORT PORT_partial_data = 0x0020; // Physical packet doesn't contain all API packet
981981
const USHORT PORT_lazy = 0x0040; // Deferred operations are allowed
982982
const USHORT PORT_server = 0x0080; // Server (not client) port
983983
const USHORT PORT_detached = 0x0100; // op_detach, op_drop_database or op_service_detach was processed
984984
const USHORT PORT_rdb_shutdown = 0x0200; // Database is shut down
985985
const USHORT PORT_connecting = 0x0400; // Aux connection waits for a channel to be activated by client
986-
const USHORT PORT_z_data = 0x0800; // Zlib incoming buffer has data left after decompression
986+
//const USHORT PORT_z_data = 0x0800; // Zlib incoming buffer has data left after decompression
987987
const USHORT PORT_compressed = 0x1000; // Compress outgoing stream (does not affect incoming)
988988
const USHORT PORT_released = 0x2000; // release(), complementary to the first addRef() in constructor, was called
989989

@@ -1041,6 +1041,9 @@ struct rem_port : public Firebird::GlobalStorage, public Firebird::RefCounted
10411041
USHORT port_protocol; // protocol version number
10421042
USHORT port_buff_size; // port buffer size
10431043
USHORT port_flags; // Misc flags
1044+
std::atomic<bool>
1045+
port_partial_data, // Physical packet doesn't contain all API packet
1046+
port_z_data; // Zlib incoming buffer has data left after decompression
10441047
SLONG port_connect_timeout; // Connection timeout value
10451048
SLONG port_dummy_packet_interval; // keep alive dummy packet interval
10461049
SLONG port_dummy_timeout; // time remaining until keepalive packet
@@ -1120,7 +1123,8 @@ struct rem_port : public Firebird::GlobalStorage, public Firebird::RefCounted
11201123
port_type(t), port_state(PENDING), port_clients(0), port_next(0),
11211124
port_parent(0), port_async(0), port_async_receive(0),
11221125
port_server(0), port_server_flags(0), port_protocol(0), port_buff_size(rpt / 2),
1123-
port_flags(0), port_connect_timeout(0), port_dummy_packet_interval(0),
1126+
port_flags(0), port_partial_data(false), port_z_data(false),
1127+
port_connect_timeout(0), port_dummy_packet_interval(0),
11241128
port_dummy_timeout(0), port_handle(INVALID_SOCKET), port_channel(INVALID_SOCKET), port_context(0),
11251129
port_events_thread(0), port_thread_guard(0),
11261130
#ifdef WIN_NT

‎src/remote/server/server.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2926,7 +2926,7 @@ void rem_port::disconnect(PACKET* sendL, PACKET* receiveL)
29262926
}
29272927

29282928
this->port_flags |= PORT_disconnect;
2929-
this->port_flags &= ~PORT_z_data;
2929+
this->port_z_data = false;
29302930

29312931
if (!rdb)
29322932
{

0 commit comments

Comments
 (0)