[2375] Provide weak exception guarantee

diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index 7cd2b4df67ef6a98eea812da601f4e5cc40c568d..62550827a919fa2362b6ed3435452fc154de14a2 100644 (file)
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -173,6 +173,9 @@ MasterLexer::getNextToken(Options options) {
         isc_throw(isc::InvalidOperation, "No source to read tokens from");
     }
     // Store current state, for the case we need to restore by ungetToken
+    impl_->has_previous_ = false;
+    // If thrown in this section, nothing observable except for not having
+    // previous will happen.
     impl_->previous_token_ = impl_->token_;
     impl_->previous_paren_count_ = impl_->paren_count_;
     impl_->previous_was_eol_ = impl_->last_was_eol_;
@@ -180,26 +183,36 @@ MasterLexer::getNextToken(Options options) {
     impl_->has_previous_ = true;
     // Reset the token now. This is to check a token was actually produced.
     // This is debugging aid.
-    impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
-    for (const State *state = start(options); state != NULL;
-         state = state->handle(*this)) {
-        // Do nothing here. All is handled in the for cycle header itself.
+
+    // From now on, if we throw, we can at least restore the old state.
+    try {
+        impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
+        for (const State *state = start(options); state != NULL;
+             state = state->handle(*this)) {
+            // Do nothing here. All is handled in the for cycle header itself.
+        }
+        // Make sure a token was produced. Since this Can Not Happen, we assert
+        // here instead of throwing.
+        assert(impl_->token_.getType() != Token::ERROR ||
+               impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
+        return (impl_->token_);
+    } catch (...) {
+        // Restore the old state and let the exception continue.
+        ungetToken();
+        throw;
     }
-    // Make sure a token was produced. Since this Can Not Happen, we assert
-    // here instead of throwing.
-    assert(impl_->token_.getType() != Token::ERROR ||
-           impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
-    return (impl_->token_);
 }
 
 void
 MasterLexer::ungetToken() {
     if (impl_->has_previous_) {
+        // The one that could fail due to bad_alloc first
+        impl_->token_ = impl_->previous_token_;
+        // The rest should not throw.
         impl_->has_previous_ = false;
         impl_->source_->ungetAll();
         impl_->last_was_eol_ = impl_->previous_was_eol_;
         impl_->paren_count_ = impl_->previous_paren_count_;
-        impl_->token_ = impl_->previous_token_;
     } else {
         isc_throw(isc::InvalidOperation, "No token to unget ready");
     }

diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index 78bcfb6ce8017640c6c9e64b5d7cc7144258dbee..6ee218933761ca40b7dd7994197638ea9bbadeb1 100644 (file)
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -183,6 +183,12 @@ public:
     /// It reads a bit of the last opened source and produces another token
     /// found in it.
     ///
+    /// This method provides the weak exception guarantee. It'll return the
+    /// class to the state before the call on exception, except that it is
+    /// not possible to call ungetToken() after the failed call. This is
+    /// considered OK, since the caller was expecting the call to succeed
+    /// in which case the previous token would not be ungettable as well.
+    ///
     /// \param options The options can be used to modify the tokenization.
     ///     The method can be made reporting things which are usually ignored
     ///     by this parameter. Multiple options can be passed at once by
@@ -192,6 +198,9 @@ public:
     /// \throw isc::InvalidOperation in case the source is not available. This
     ///     may mean the pushSource() has not been called yet, or that the
     ///     current source has been read past the end.
+    /// \throw isc::master_lexer_internal::InputSource::ReadError in case
+    ///     there's problem reading from the underlying source (eg. I/O error
+    ///     in the file on the disk).
     /// \throw std::bad_alloc in case allocation of some internal resources
     ///     or the token fail.
     Token getNextToken(Options options = NONE);

diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index 2e98cabd1fdc3f07271be808a9146f15e1bcc7e3..b02ae70cd28c1d48f7728afad3b041d8d24867a2 100644 (file)
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -384,4 +384,40 @@ TEST_F(MasterLexerTest, ungetAfterSwitch) {
     EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
 }
 
+class TestException {};
+
+void
+doThrow(const std::string&) {
+    throw TestException();
+}
+
+// Check the getNextToken provides at least the weak exception guarantee.
+TEST_F(MasterLexerTest, getTokenExceptions) {
+    ss << "\n12345";
+    lexer.pushSource(ss);
+
+    // Prepare a chain that changes the internal state, reads something.
+    // The next item in the chain will throw an exception (we explicitly
+    // throw something not known to it, so we know it can handle anything).
+    // Then the thing should get to the previous state and getting the
+    // token the usual way without mock should work.
+    const bool true_value = true;
+    boost::scoped_ptr<State> s2(State::getFakeState(NULL, 3, NULL, 0, NULL,
+                                                    &doThrow));
+    boost::scoped_ptr<State> s1(State::getFakeState(s2.get(), 3, NULL, 1,
+                                                    &true_value));
+    lexer.pushFakeStart(s1.get());
+
+    // Getting the token with the fake start should throw. But then, the
+    // current state should be untouched.
+    EXPECT_THROW(lexer.getNextToken(), TestException);
+    EXPECT_EQ(0, s1->getParenCount(lexer));
+    EXPECT_FALSE(s1->wasLastEOL(lexer));
+    EXPECT_EQ(MasterLexer::Token::NOT_STARTED,
+              s1->getToken(lexer).getErrorCode());
+
+    // It gets back to the original state, so getting the newline works.
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+}
+
 }