From 7a0a6ec04debd09d2f6d14612beb14ce78b93ed1 Mon Sep 17 00:00:00 2001 From: Chenhao Qu Date: Wed, 22 Sep 2021 06:58:41 +0000 Subject: Import wiredtiger: 1bbc53179f939c3a1cd446a4bc850d0b10a2d74e from branch mongodb-master ref: f7722dec10..1bbc53179f for: 5.1.0 WT-8108 Inconsistent tiered file size WT-8112 Coverity analysis defect 120743: Dereference before null check WT-8113 Coverity analysis defect 120744: Logically dead code WT-8126 Mark btree as dirty only if not newly created when instantiating a deleted row-store leaf page WT-8148 Fix comment typo in util_verify.c --- .../wiredtiger/ext/extractors/csv/csv_extractor.c | 7 +- .../ext/storage_sources/local_store/local_store.c | 39 +++++++---- src/third_party/wiredtiger/import.data | 2 +- src/third_party/wiredtiger/src/btree/bt_delete.c | 10 +-- .../wiredtiger/src/utilities/util_verify.c | 2 +- .../wiredtiger/test/suite/test_verify2.py | 76 ++++++++++++++++++++++ 6 files changed, 112 insertions(+), 24 deletions(-) create mode 100755 src/third_party/wiredtiger/test/suite/test_verify2.py diff --git a/src/third_party/wiredtiger/ext/extractors/csv/csv_extractor.c b/src/third_party/wiredtiger/ext/extractors/csv/csv_extractor.c index 72231d388ea..746ca396298 100644 --- a/src/third_party/wiredtiger/ext/extractors/csv/csv_extractor.c +++ b/src/third_party/wiredtiger/ext/extractors/csv/csv_extractor.c @@ -167,10 +167,6 @@ csv_customize(WT_EXTRACTOR *extractor, WT_SESSION *session, const char *uri, WT_ wt_api->strerror(wt_api, session, ret)); goto err; } - if (ret != 0) { - (void)wt_api->err_printf( - wt_api, session, "WT_CONFIG_PARSER.close: %s", wt_api->strerror(wt_api, session, ret)); - } field_num = strtol(field.str, NULL, 10); if (field_num < 0 || field_num > INT_MAX) { @@ -195,8 +191,7 @@ csv_customize(WT_EXTRACTOR *extractor, WT_SESSION *session, const char *uri, WT_ return (0); err: - if (parser != NULL) - (void)parser->close(parser); + (void)parser->close(parser); return (ret); } diff --git a/src/third_party/wiredtiger/ext/storage_sources/local_store/local_store.c b/src/third_party/wiredtiger/ext/storage_sources/local_store/local_store.c index 3184de0da3b..8bcb6911d06 100644 --- a/src/third_party/wiredtiger/ext/storage_sources/local_store/local_store.c +++ b/src/third_party/wiredtiger/ext/storage_sources/local_store/local_store.c @@ -176,10 +176,6 @@ static int local_file_size(WT_FILE_HANDLE *, WT_SESSION *, wt_off_t *); static int local_file_sync(WT_FILE_HANDLE *, WT_SESSION *); static int local_file_write(WT_FILE_HANDLE *, WT_SESSION *, wt_off_t, size_t, const void *); -/* - * Report an error for a file operation. Note that local_err returns its third argument, and this - * macro will too. - */ #define FS2LOCAL(fs) (((LOCAL_FILE_SYSTEM *)(fs))->local_storage) #define SHOW_STRING(s) (((s) == NULL) ? "" : (s)) #define VERBOSE_LS(local, ...) \ @@ -285,7 +281,7 @@ local_err(LOCAL_STORAGE *local, WT_SESSION *session, int ret, const char *format va_start(ap, format); wt_api = local->wt_api; - if (vsnprintf(buf, sizeof(buf), format, ap) > (int)sizeof(buf)) + if (vsnprintf(buf, sizeof(buf), format, ap) >= (int)sizeof(buf)) wt_api->err_printf(wt_api, session, "local_storage: error overflow"); wt_api->err_printf( wt_api, session, "local_storage: %s: %s", wt_api->strerror(wt_api, session, ret), buf); @@ -317,7 +313,8 @@ local_get_directory(const char *home, const char *s, ssize_t len, bool create, c else { buflen = (size_t)len + strlen(home) + 2; /* Room for slash, null */ if ((dirname = malloc(buflen)) != NULL) - snprintf(dirname, buflen, "%s/%.*s", home, (int)len, s); + if (snprintf(dirname, buflen, "%s/%.*s", home, (int)len, s) >= (int)buflen) + return (EINVAL); } if (dirname == NULL) return (ENOMEM); @@ -382,7 +379,8 @@ local_path(WT_FILE_SYSTEM *file_system, const char *dir, const char *name, char len = strlen(dir) + strlen(name) + 2; if ((p = malloc(len)) == NULL) return (local_err(FS2LOCAL(file_system), NULL, ENOMEM, "local_path")); - snprintf(p, len, "%s/%s", dir, name); + if (snprintf(p, len, "%s/%s", dir, name) >= (int)len) + return (local_err(FS2LOCAL(file_system), NULL, EINVAL, "overflow sprintf")); *pathp = p; return (ret); } @@ -526,7 +524,10 @@ local_customize_file_system(WT_STORAGE_SOURCE *storage_source, WT_SESSION *sessi p++; else p = bucket_name; - snprintf(buf, sizeof(buf), "cache-%s", p); + if (snprintf(buf, sizeof(buf), "cache-%s", p) >= (int)sizeof(buf)) { + ret = local_err(local, session, EINVAL, "overflow snprintf"); + goto err; + } cachedir.str = buf; cachedir.len = strlen(buf); } @@ -593,10 +594,17 @@ local_file_copy(LOCAL_STORAGE *local, WT_SESSION *session, const char *src_path, WT_FILE_SYSTEM *wt_fs; wt_off_t copy_size, file_size, left; ssize_t pos; + size_t pathlen; int ret, t_ret; - char buffer[1024 * 64]; + char buffer[1024 * 64], *tmp_path; dest = src = NULL; + pathlen = strlen(dest_path) + 10; + if ((tmp_path = malloc(pathlen)) != NULL) + if (snprintf(tmp_path, pathlen, "%s.TMP", dest_path) >= (int)pathlen) { + ret = local_err(local, session, EINVAL, "overflow snprintf"); + goto err; + } if ((ret = local->wt_api->file_system_get(local->wt_api, session, &wt_fs)) != 0) { ret = @@ -609,9 +617,9 @@ local_file_copy(LOCAL_STORAGE *local, WT_SESSION *session, const char *src_path, goto err; } - if ((ret = wt_fs->fs_open_file(wt_fs, session, dest_path, type, WT_FS_OPEN_CREATE, &dest)) != + if ((ret = wt_fs->fs_open_file(wt_fs, session, tmp_path, type, WT_FS_OPEN_CREATE, &dest)) != 0) { - ret = local_err(local, session, ret, "%s: cannot create", dest_path); + ret = local_err(local, session, ret, "%s: cannot create", tmp_path); goto err; } if ((ret = wt_fs->fs_size(wt_fs, session, src_path, &file_size)) != 0) { @@ -625,10 +633,14 @@ local_file_copy(LOCAL_STORAGE *local, WT_SESSION *session, const char *src_path, goto err; } if ((ret = dest->fh_write(dest, session, pos, (size_t)copy_size, buffer)) != 0) { - ret = local_err(local, session, ret, "%s: cannot write", dest_path); + ret = local_err(local, session, ret, "%s: cannot write", tmp_path); goto err; } } + if ((ret = rename(tmp_path, dest_path)) != 0) { + ret = local_err(local, session, errno, "%s: cannot rename from %s", dest_path, tmp_path); + goto err; + } err: if (src != NULL && (t_ret = src->close(src, session)) != 0) if (ret == 0) @@ -636,6 +648,9 @@ err: if (dest != NULL && (t_ret = dest->close(dest, session)) != 0) if (ret == 0) ret = t_ret; + if (ret != 0) + (void)unlink(tmp_path); + free(tmp_path); return (ret); } diff --git a/src/third_party/wiredtiger/import.data b/src/third_party/wiredtiger/import.data index 7941c65a6ff..3ab3ce188ab 100644 --- a/src/third_party/wiredtiger/import.data +++ b/src/third_party/wiredtiger/import.data @@ -2,5 +2,5 @@ "vendor": "wiredtiger", "github": "wiredtiger/wiredtiger.git", "branch": "mongodb-master", - "commit": "f7722dec100b659418f00d8e338ce1fc35d489de" + "commit": "1bbc53179f939c3a1cd446a4bc850d0b10a2d74e" } diff --git a/src/third_party/wiredtiger/src/btree/bt_delete.c b/src/third_party/wiredtiger/src/btree/bt_delete.c index acb89293002..f35346db415 100644 --- a/src/third_party/wiredtiger/src/btree/bt_delete.c +++ b/src/third_party/wiredtiger/src/btree/bt_delete.c @@ -305,12 +305,14 @@ __wt_delete_page_instantiate(WT_SESSION_IMPL *session, WT_REF *ref) /* * Give the page a modify structure. - * - * Mark tree dirty, unless the handle is read-only. (We'd like to free the deleted pages, but if - * the handle is read-only, we're not able to do so.) */ WT_RET(__wt_page_modify_init(session, page)); - if (!F_ISSET(btree, WT_BTREE_READONLY)) + + /* + * We would like to free the deleted pages, but if the tree is newly created, there is nothing + * that needs to be freed. Furthermore, if the handle is read-only, we are not able to do so. + */ + if (!btree->original) __wt_page_modify_set(session, page); /* diff --git a/src/third_party/wiredtiger/src/utilities/util_verify.c b/src/third_party/wiredtiger/src/utilities/util_verify.c index 910c03aa629..1ee4c1bcc90 100644 --- a/src/third_party/wiredtiger/src/utilities/util_verify.c +++ b/src/third_party/wiredtiger/src/utilities/util_verify.c @@ -67,7 +67,7 @@ util_verify(WT_SESSION *session, int argc, char *argv[]) /* * The remaining argument is the table name. If we are verifying the history store we do not - * accept a URI. Otherwise, we need a URI top operate on. + * accept a URI. Otherwise, we need a URI to operate on. */ if (argc != 1) return (usage()); diff --git a/src/third_party/wiredtiger/test/suite/test_verify2.py b/src/third_party/wiredtiger/test/suite/test_verify2.py new file mode 100755 index 00000000000..65163191b8f --- /dev/null +++ b/src/third_party/wiredtiger/test/suite/test_verify2.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python +# +# Public Domain 2014-present MongoDB, Inc. +# Public Domain 2008-2014 WiredTiger, Inc. +# +# This is free and unencumbered software released into the public domain. +# +# Anyone is free to copy, modify, publish, use, compile, sell, or +# distribute this software, either in source code form or as a compiled +# binary, for any purpose, commercial or non-commercial, and by any +# means. +# +# In jurisdictions that recognize copyright laws, the author or authors +# of this software dedicate any and all copyright interest in the +# software to the public domain. We make this dedication for the benefit +# of the public at large and to the detriment of our heirs and +# successors. We intend this dedication to be an overt act of +# relinquishment in perpetuity of all present and future rights to this +# software under copyright law. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +# IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR +# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +# OTHER DEALINGS IN THE SOFTWARE. + +import wiredtiger, wttest +class test_verify2(wttest.WiredTigerTestCase): + tablename = 'test_verify' + params = 'key_format=S,value_format=S' + uri = 'table:' + tablename + + # Create an empty table and insert content. + # The first call to verify is expected to return to EBUSY due to the dirty content. Call + # checkpoint to make the table clean, the next verify call should succeed. + def test_verify_ckpt(self): + self.assertEqual(self.session.create(self.uri, self.params), 0) + self.assertEqual(self.conn.set_timestamp('stable_timestamp=' + self.timestamp_str(10)), 0) + + # Insert data. + cursor = self.session.open_cursor(self.uri) + cursor["0"] = "000" + cursor.close() + + # Calling verify without checkpointing before will return EBUSY because of the dirty data. + self.assertTrue(self.raisesBusy(lambda: self.session.verify(self.uri, None)), + "was expecting API call to fail with EBUSY") + + # Checkpointing will get rid of the dirty data. + self.assertEqual(self.session.checkpoint(), 0) + + # Verify. + self.assertEqual(self.session.verify(self.uri, None), 0) + + # Create an empty table and search a key. This used to mark the associated btree as dirty. In + # fact, because the tree is empty, its only reference to a leaf page is marked as deleted and we + # instantiate the deleted page in this case. Before WT-8126, this would mark the btree as + # modified. + def test_verify_search(self): + self.assertEqual(self.session.create(self.uri, self.params), 0) + self.assertEqual(self.conn.set_timestamp('stable_timestamp=' + self.timestamp_str(10)), 0) + + # Search for data. + cursor = self.session.open_cursor(self.uri) + cursor.set_key("1") + self.assertEqual(cursor.search(), wiredtiger.WT_NOTFOUND) + cursor.close() + + # We don't need to call checkpoint before calling verify as the btree is not marked as + # modified. + self.assertEqual(self.session.verify(self.uri, None), 0) + +if __name__ == '__main__': + wttest.run() -- cgit v1.2.1