BIND 10 trac931, updated. f8e2bf68f643abc26b6f6dd2613b3ef0c2dc4a73 [trac931] Fix double reads
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed May 18 21:46:19 UTC 2011
The branch, trac931 has been updated
via f8e2bf68f643abc26b6f6dd2613b3ef0c2dc4a73 (commit)
via 858f0735c6b4c2d71ee32640fbf8b9276e8320ed (commit)
via 5df12a26d3886a10f069ac5e1fd5c4eb2106a372 (commit)
via 3ff8accf4a7b4fe7b54d4c23e1e6a8406a315bf0 (commit)
from a8307030f7af9fc88e3e66b6eefcc89f6b6e15c5 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit f8e2bf68f643abc26b6f6dd2613b3ef0c2dc4a73
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 23:10:29 2011 +0200
[trac931] Fix double reads
When we accidentally scheduled two reads of message length, it did bad
stuff, like reading one length, reading next 4 bytes of the message in
the second call and calling the handler after that.
This solution is probably not completely clean, but it works. It tracks
if something scheduled an asynchronous read and if it did, we don't
schedule another one. In such case, we let it do the reads for us, only
checking the queued messages.
commit 858f0735c6b4c2d71ee32640fbf8b9276e8320ed
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 23:05:44 2011 +0200
[trac931] Use uint32_t for fixed-sized lengths
The length sent over wire are 32bit unsigned integers, but it could read
it as size_t which might be 64bit. It's strange this didn't cause any
trouble, but this is cleaner anyway.
commit 5df12a26d3886a10f069ac5e1fd5c4eb2106a372
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 22:59:55 2011 +0200
[trac931] Send the correct name of module
commit 3ff8accf4a7b4fe7b54d4c23e1e6a8406a315bf0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 22:55:39 2011 +0200
[trac931] Fix sending spec from the manager
-----------------------------------------------------------------------
Summary of changes:
src/lib/cc/session.cc | 96 ++++++++++++++++++++++++-----------
src/lib/config/ccsession.cc | 2 +-
src/lib/python/isc/config/cfgmgr.py | 5 +-
3 files changed, 70 insertions(+), 33 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/cc/session.cc b/src/lib/cc/session.cc
index e911a86..f037b00 100644
--- a/src/lib/cc/session.cc
+++ b/src/lib/cc/session.cc
@@ -73,17 +73,17 @@ namespace cc {
class SessionImpl {
public:
SessionImpl(io_service& io_service) :
- sequence_(-1), queue_(Element::createList()),
+ sequence_(-1), queue_(Element::createList()), read_pending_(false),
io_service_(io_service), socket_(io_service_), data_length_(0),
timeout_(MSGQ_DEFAULT_TIMEOUT)
{}
void establish(const char& socket_file);
void disconnect();
- void writeData(const void* data, size_t datalen);
- size_t readDataLength();
+ void writeData(const void* data, uint32_t datalen);
+ uint32_t readDataLength();
// Blocking read. Will throw a SessionTimeout if the timeout value
// (in seconds) is thrown. If timeout is 0 it will block forever
- void readData(void* data, size_t datalen);
+ void readData(void* data, uint32_t datalen);
void startRead(boost::function<void()> user_handler);
void setTimeout(size_t seconds) { timeout_ = seconds; };
size_t getTimeout() const { return timeout_; };
@@ -91,16 +91,23 @@ public:
long int sequence_; // the next sequence number to use
std::string lname_;
ElementPtr queue_;
+ bool read_pending_;
private:
void internalRead(const asio::error_code& error,
- size_t bytes_transferred);
+ uint32_t bytes_transferred);
private:
io_service& io_service_;
+public:
asio::local::stream_protocol::socket socket_;
+private:
uint32_t data_length_;
boost::function<void()> user_handler_;
+ // If we schedule one read of the message length, we can't schedule
+ // another one, or the second read would read from the real message
+ // and then call the handlers afterwards. So we note if some read is
+ // pending and wait for the one to happen first.
asio::error_code error_;
size_t timeout_;
@@ -136,7 +143,7 @@ SessionImpl::disconnect() {
}
void
-SessionImpl::writeData(const void* data, size_t datalen) {
+SessionImpl::writeData(const void* data, uint32_t datalen) {
try {
asio::write(socket_, asio::buffer(data, datalen));
} catch (const asio::system_error& asio_ex) {
@@ -144,9 +151,10 @@ SessionImpl::writeData(const void* data, size_t datalen) {
}
}
-size_t
+uint32_t
SessionImpl::readDataLength() {
- size_t ret_len = data_length_;
+ uint32_t ret_len = data_length_;
+ assert(!read_pending_);
if (ret_len == 0) {
readData(&data_length_, sizeof(data_length_));
@@ -161,7 +169,7 @@ SessionImpl::readDataLength() {
}
void
-SessionImpl::readData(void* data, size_t datalen) {
+SessionImpl::readData(void* data, uint32_t datalen) {
boost::optional<asio::error_code> read_result;
boost::optional<asio::error_code> timer_result;
@@ -216,6 +224,8 @@ SessionImpl::readData(void* data, size_t datalen) {
void
SessionImpl::startRead(boost::function<void()> user_handler) {
+ // A read is pending right now, don't schedule another one
+ read_pending_ = true;
data_length_ = 0;
user_handler_ = user_handler;
asio::async_read(socket_, asio::buffer(&data_length_,
@@ -227,8 +237,10 @@ SessionImpl::startRead(boost::function<void()> user_handler) {
void
SessionImpl::internalRead(const asio::error_code& error,
- size_t bytes_transferred)
+ uint32_t bytes_transferred)
{
+ // We finished the read now
+ read_pending_ = false;
if (!error) {
assert(bytes_transferred == sizeof(data_length_));
data_length_ = ntohl(data_length_);
@@ -349,29 +361,46 @@ bool
Session::recvmsg(ConstElementPtr& env, ConstElementPtr& msg,
bool nonblock, int seq)
{
- size_t length = impl_->readDataLength();
- if (hasQueuedMsgs()) {
- ConstElementPtr q_el;
- for (int i = 0; i < impl_->queue_->size(); i++) {
- q_el = impl_->queue_->get(i);
- if (( seq == -1 &&
- !q_el->get(0)->contains("reply")
- ) || (
- q_el->get(0)->contains("reply") &&
- q_el->get(0)->get("reply")->intValue() == seq
- )
- ) {
- env = q_el->get(0);
- msg = q_el->get(1);
- impl_->queue_->remove(i);
- return (true);
+ do {
+ if (hasQueuedMsgs()) {
+ ConstElementPtr q_el;
+ for (int i = 0; i < impl_->queue_->size(); i++) {
+ q_el = impl_->queue_->get(i);
+ if (( seq == -1 &&
+ !q_el->get(0)->contains("reply")
+ ) || (
+ q_el->get(0)->contains("reply") &&
+ q_el->get(0)->get("reply")->intValue() == seq
+ )
+ ) {
+ env = q_el->get(0);
+ msg = q_el->get(1);
+ impl_->queue_->remove(i);
+ return (true);
+ }
}
}
- }
-
+
+ // If a read is pending somewhere, it's pending on something
+ // that reads messages. So we can't register our read, but
+ // we'll probably get some queued messages after a while.
+ if (impl_->read_pending_) {
+ // If we shouldn't block, we just return false, there's no
+ // message for us right now and we let him to read whatever's
+ // there.
+ //
+ // This is not complete nonblock support, though
+ if (nonblock) {
+ return (false);
+ }
+
+ impl_->socket_.io_service().run_one();
+ }
+ } while (impl_->read_pending_);
+
+ uint32_t length = impl_->readDataLength();
unsigned short header_length_net;
impl_->readData(&header_length_net, sizeof(header_length_net));
-
unsigned short header_length = ntohs(header_length_net);
if (header_length > length || length < 2) {
isc_throw(SessionError, "Length parameters invalid: total=" << length
@@ -410,7 +439,14 @@ Session::recvmsg(ConstElementPtr& env, ConstElementPtr& msg,
q_el->add(l_env);
q_el->add(l_msg);
impl_->queue_->add(q_el);
- return (recvmsg(env, msg, nonblock, seq));
+ // This is a hack for now. This is not real non-blocking, but
+ // as the data might have came a little bit later, it should
+ // be functionally the same. The caller will try us later again.
+ if (nonblock) {
+ return (false);
+ } else {
+ return (recvmsg(env, msg, nonblock, seq));
+ }
}
// XXXMLG handle non-block here, and return false for short reads
}
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 81eadab..d7f14f4 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -372,7 +372,7 @@ ModuleCCSession::addRemoteConfig(const std::string& spec_name,
ConstElementPtr cmd = Element::fromJSON("{ \"command\": ["
"\"get_module_spec\","
"{\"module_name\": \"" +
- module_name + "\"} ] }");
+ spec_name + "\"} ] }");
unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
ConstElementPtr env, answer;
session_.group_recvmsg(env, answer, false, seq);
diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py
index 88a93e1..ffdafcd 100644
--- a/src/lib/python/isc/config/cfgmgr.py
+++ b/src/lib/python/isc/config/cfgmgr.py
@@ -217,7 +217,7 @@ class ConfigManager:
return self.module_specs[module_name]
else:
# TODO: log error?
- return {}
+ return isc.config.ModuleSpec({}, False)
else:
result = {}
for module in self.module_specs:
@@ -272,7 +272,8 @@ class ConfigManager:
if type(cmd) == dict:
if 'module_name' in cmd and cmd['module_name'] != '':
module_name = cmd['module_name']
- answer = ccsession.create_answer(0, self.get_module_spec(module_name))
+ answer = ccsession.create_answer(0,
+ self.get_module_spec(module_name).get_full_spec())
else:
answer = ccsession.create_answer(1, "Bad module_name in get_module_spec command")
else:
More information about the bind10-changes
mailing list