Merge pull request #135 from kspangsege/ks-cleanup

Bug fixes, documentation (source level), and general cleanup

changelog.txt

@@ -12,9 +12,20 @@ Format:
 
 ==================
 
+
+2013-08-23 (Kristian Spangsege)
++ Stop throwing from destructors (all), and from SharedGroup::rollback() and SharedGRoup::end_read().
++ General stability and eror checking improvements.
+! Fixed many bugs relating to Group::commit().
+! Fixed some bugs relating to SharedGroup::commit().
+! Fixed bug in TableViewBase::sort().
+
+
+
 2013-08-18 (Kenneth Geisshirt)
 ! Group::to_string formatting was incorrect. See https://app.asana.com/0/1441391972580/5659532773181.
 
+
 2013-08-03 (Kristian Spangsege)
 + Table::find_sorted_int() replaced by Table::lower_bound_int() and Table::upper_bound_int() as these are standardized and provide more flexibility.
 + Addition of Table::lower_bound_bool() and Table::upper_bound_bool().

src/tightdb/Makefile

@@ -1,11 +1,11 @@
 GENERATED_SOURCES = table_macros.hpp
 
 INST_HEADERS = config.h meta.hpp assert.hpp exceptions.hpp terminate.hpp type_list.hpp tuple.hpp \
-bind.hpp safe_int_ops.hpp unique_ptr.hpp bind_ptr.hpp string_buffer.hpp file.hpp thread.hpp \
-utf8.hpp utilities.hpp alloc.hpp alloc_slab.hpp array.hpp array_string.hpp data_type.hpp \
-column_type.hpp column_fwd.hpp spec.hpp date.hpp string_data.hpp binary_data.hpp mixed.hpp \
-table.hpp table_ref.hpp table_basic_fwd.hpp table_accessors.hpp table_basic.hpp table_view.hpp \
-table_view_basic.hpp table_macros.hpp group.hpp group_shared.hpp query.hpp \
+bind.hpp safe_int_ops.hpp unique_ptr.hpp bind_ptr.hpp buffer.hpp string_buffer.hpp file.hpp \
+thread.hpp utf8.hpp utilities.hpp alloc.hpp alloc_slab.hpp array.hpp array_string.hpp \
+data_type.hpp column_type.hpp column_fwd.hpp spec.hpp date.hpp string_data.hpp binary_data.hpp \
+mixed.hpp table.hpp table_ref.hpp table_basic_fwd.hpp table_accessors.hpp table_basic.hpp \
+table_view.hpp table_view_basic.hpp table_macros.hpp group.hpp group_shared.hpp query.hpp \
 query_conditions.hpp lang_bind_helper.hpp tightdb_nmmintrin.h replication.hpp
 
 lib_LIBRARIES = libtightdb.a

src/tightdb/alloc.cpp

@@ -1,38 +1,72 @@
 #include <cerrno>
 #include <cstdlib>
 #include <stdexcept>
+#include <algorithm>
 
 #include <tightdb/alloc_slab.hpp>
 
 using namespace std;
 using namespace tightdb;
 
 
+// FIXME: Casting a pointers to std::size_t is inherently
+// nonportable. For example, systems exist where pointers are 64 bits
+// and std::size_t is 32. One idea would be to use a different type
+// for refs such as std::uintptr_t, the problem with this one is that
+// while it is described by the C++11 standard it is not required to
+// be present. C++03 does not even mention it. A real working solution
+// will be to introduce a new name for the type of refs. The typedef
+// can then be made as complex as required to pick out an appropriate
+// type on any supported platform.
+//
+// A better solution may be to use an instance of SlabAlloc. The main
+// problem is that SlabAlloc is not thread-safe. Another problem is
+// that its free-list management is currently exceedingly slow do to
+// linear searches. Another problem is that it is prone to general
+// memory corruption due to lack of exception safety when upding
+// free-lists. But these problems must be fixed anyway.
+
+
 namespace {
 
-// For use with free-standing objects (objects that are not part of a
-// TightDB group)
+/// For use with free-standing objects (objects that are not part of a
+/// TightDB group)
+///
+/// Note that it is essential that this class is stateless as it may
+/// be used by multiple threads. Although it has m_replication, this
+/// is not a problem, as there is no way to modify it, so it will
+/// remain zero.
 class DefaultAllocator: public tightdb::Allocator {
 public:
     MemRef alloc(size_t size) TIGHTDB_OVERRIDE
     {
         char* addr = static_cast<char*>(malloc(size));
-        if (TIGHTDB_LIKELY(addr))
-            return MemRef(addr, reinterpret_cast<size_t>(addr));
-        TIGHTDB_ASSERT(errno == ENOMEM);
-        throw bad_alloc();
+        if (TIGHTDB_UNLIKELY(!addr)) {
+            TIGHTDB_ASSERT(errno == ENOMEM);
+            throw bad_alloc();
+        }
+#ifdef TIGHTDB_ALLOC_SET_ZERO
+        fill(addr, addr+size, 0);
+#endif
+        return MemRef(addr, reinterpret_cast<size_t>(addr));
     }
 
-    MemRef realloc_(ref_type, const char* addr, size_t size) TIGHTDB_OVERRIDE
+    MemRef realloc_(ref_type, const char* addr, size_t old_size, size_t new_size) TIGHTDB_OVERRIDE
     {
-        char* new_addr = static_cast<char*>(realloc(const_cast<char*>(addr), size));
-        if (TIGHTDB_LIKELY(new_addr))
-            return MemRef(new_addr, reinterpret_cast<size_t>(new_addr));
-        TIGHTDB_ASSERT(errno == ENOMEM);
-        throw bad_alloc();
+        char* new_addr = static_cast<char*>(realloc(const_cast<char*>(addr), new_size));
+        if (TIGHTDB_UNLIKELY(!new_addr)) {
+            TIGHTDB_ASSERT(errno == ENOMEM);
+            throw bad_alloc();
+        }
+#ifdef TIGHTDB_ALLOC_SET_ZERO
+        fill(new_addr+old_size, new_addr+new_size, 0);
+#else
+        static_cast<void>(old_size);
+#endif
+        return MemRef(new_addr, reinterpret_cast<size_t>(new_addr));
     }
 
-    void free_(ref_type, const char* addr) TIGHTDB_OVERRIDE
+    void free_(ref_type, const char* addr) TIGHTDB_NOEXCEPT TIGHTDB_OVERRIDE
     {
         free(const_cast<char*>(addr));
     }
@@ -58,6 +92,6 @@ class DefaultAllocator: public tightdb::Allocator {
 
 Allocator& Allocator::get_default() TIGHTDB_NOEXCEPT
 {
-    static DefaultAllocator alloc;
-    return alloc;
+    static DefaultAllocator default_alloc;
+    return default_alloc;
 }

src/tightdb/alloc.hpp

@@ -50,17 +50,6 @@ struct MemRef {
     ref_type m_ref;
 };
 
-// FIXME: Casting a pointer to std::size_t is inherently nonportable
-// (see the default definition of Allocator::alloc()). For example,
-// systems exist where pointers are 64 bits and std::size_t is 32. One
-// idea would be to use a different type for refs such as
-// std::uintptr_t, the problem with this one is that while it is
-// described by the C++11 standard it is not required to be
-// present. C++03 does not even mention it. A real working solution
-// will be to introduce a new name for the type of refs. The typedef
-// can then be made as complex as required to pick out an appropriate
-// type on any supported platform.
-
 
 /// The common interface for TightDB allocators.
 ///
@@ -87,16 +76,16 @@ class Allocator {
     ///
     /// \throw std::bad_alloc If insufficient memory was available.
     ///
-    /// Note: The underscore was added because the name \c realloc
+    /// Note: The underscore has been added because the name `realloc`
     /// would conflict with a macro on the Windows platform.
-    virtual MemRef realloc_(ref_type ref, const char* addr, std::size_t size) = 0;
+    virtual MemRef realloc_(ref_type ref, const char* addr, std::size_t old_size,
+                            std::size_t new_size) = 0;
 
-    // FIXME: SlabAlloc::free_() should be modified such than this
-    // method never throws.
+    /// Release the specified chunk of memory.
     ///
-    /// Note: The underscore was added because the name \c free would
-    /// conflict with a macro on the Windows platform.
-    virtual void free_(ref_type, const char* addr) = 0;
+    /// Note: The underscore has been added because the name `free
+    /// would conflict with a macro on the Windows platform.
+    virtual void free_(ref_type, const char* addr) TIGHTDB_NOEXCEPT = 0;
 
     /// Map the specified \a ref to the corresponding memory
     /// address. Note that if is_read_only(ref) returns true, then the
@@ -118,7 +107,7 @@ class Allocator {
     /// therefore, is not part of an actual database.
     static Allocator& get_default() TIGHTDB_NOEXCEPT;
 
-    virtual ~Allocator() {}
+    virtual ~Allocator() TIGHTDB_NOEXCEPT {}
 
 #ifdef TIGHTDB_DEBUG
     virtual void Verify() const = 0;