Thrift: Make borrow (almost) always succeed for TBufferedTransport.
Chad Walters is writing a JSON protocol for Thrift, but he wants
borrow to always succeed. That would be a pain to implement,
but here is a first step: borrow will almost always work with
TBufferedTransport.
Reviewed by: mcslee
Test Plan: Ran the DenseProtocol test and Zlib test, but more needs to be done.
Other Notes:
Also reviewed by Chad Walters, and maybe Ben Maurer.
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665455 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/cpp/src/transport/TTransportUtils.cpp b/lib/cpp/src/transport/TTransportUtils.cpp
index b5e7fa8..556d5fa 100644
--- a/lib/cpp/src/transport/TTransportUtils.cpp
+++ b/lib/cpp/src/transport/TTransportUtils.cpp
@@ -60,14 +60,42 @@
}
const uint8_t* TBufferedTransport::borrow(uint8_t* buf, uint32_t* len) {
- // Don't try to be clever with shifting buffers.
- // If we have enough data, give a pointer to it,
- // otherwise let the protcol use its slow path.
- if (rLen_-rPos_ >= *len) {
+ // The number of additional bytes we need from the underlying transport.
+ // Could be zero or negative.
+ uint32_t need = *len - (rLen_-rPos_);
+
+ // If we have enough data, just hand over a pointer.
+ if (need <= 0) {
*len = rLen_-rPos_;
return rBuf_+rPos_;
}
- return NULL;
+
+ // If the request is bigger than our buffer, we are hosed.
+ if (*len > rBufSize_) {
+ return NULL;
+ }
+
+ // If we have less than half our buffer available,
+ // or we need more space than is in the buffer,
+ // shift the data we have down to the start.
+ if ((rLen_ > rBufSize_/2) || (rLen_+need > rBufSize_)) {
+ memmove(rBuf_, rBuf_+rPos_, rLen_-rPos_);
+ rLen_ -= rPos_;
+ rPos_ = 0;
+ }
+
+ // First try to fill up the buffer.
+ uint32_t got = transport_->read(rBuf_+rLen_, rBufSize_-rLen_);
+ rLen_ += got;
+ need -= got;
+
+ // If that fails, readAll until we get what we need.
+ if (need > 0) {
+ rLen_ += transport_->readAll(rBuf_+rLen_, need);
+ }
+
+ *len = rLen_-rPos_;
+ return rBuf_+rPos_;
}
void TBufferedTransport::consume(uint32_t len) {
diff --git a/lib/cpp/src/transport/TTransportUtils.h b/lib/cpp/src/transport/TTransportUtils.h
index ed64dec..6aceb82 100644
--- a/lib/cpp/src/transport/TTransportUtils.h
+++ b/lib/cpp/src/transport/TTransportUtils.h
@@ -106,6 +106,17 @@
void flush();
+ /**
+ * The following behavior is currently implemented by TBufferedTransport,
+ * but that may change in a future version:
+ * 1/ If len is at most rBufSize_, borrow will never return NULL.
+ * Depending on the underlying transport, it could throw an exception
+ * or hang forever.
+ * 2/ Some borrow requests may copy bytes internally. However,
+ * if len is at most rBufSize_/2, none of the copied bytes
+ * will ever have to be copied again. For optimial performance,
+ * stay under this limit.
+ */
const uint8_t* borrow(uint8_t* buf, uint32_t* len);
void consume(uint32_t len);