summaryrefslogtreecommitdiff
path: root/src/btree
diff options
context:
space:
mode:
authorKeith Bostic <keith.bostic@mongodb.com>2016-07-11 11:23:05 -0400
committerGitHub <noreply@github.com>2016-07-11 11:23:05 -0400
commitf258ae5ac12a0394c8c8559fe6aeefff06092266 (patch)
tree1f202b33aabdf0863210b73938945372f895c45e /src/btree
parent1c67c4e0f011f2a7f7ca582b4c2e6f6154ac3b20 (diff)
downloadmongo-f258ae5ac12a0394c8c8559fe6aeefff06092266.tar.gz
WT-2693 Check open_cursor error paths for consistent handling (#2859)
* WT-2693 Check open_cursor error paths for consistent handling There's a problem with cursor functions which don't call their typed close functions, relying instead on a simple free of the base cursor type because the __wt_cursor_init() function is the last function called before successful return. The problem is __wt_cursor_init() doesn't clean up after itself: for example, it allocates memory to hold the URI in WT_CURSOR.internal_uri, then has a number of subsequent potential error returns, and WT_CURSOR.internal_uri won't be free'd unless __wt_cursor_close() is called. In other words, we should always call the typed closed function (which in turn calls __wt_cursor_close()). Specific additional concerns: cursor/cur_backup.c: __wt_curbackup_open() doesn't turn off backups or clear the hot-backup flag if __wt_cursor_init() fails, which will hang the process. Remove the cleanup-on-error code from the __backup_start() function and rely instead on the backup cursor's close function to do the work. There's a problem if we fail the initial backup cursor test, in which case we don't want to do cleanup other than discarding the cursors themselves. Add the WT_CURBACKUP_LOCKER flag to the WT_CURSOR_BACKUP structure, which isn't set until we're holding the hot backup cursor flag, and use it to figure out how much cleanup needs to be done. cursor/cur_file.c: the change to call the __curfile_close() function has a problem with __wt_bulk_wrapup(), which assumes WT_CURSOR_BULK.reconcile has been initialized, change it to check for NULL. cursor/cur_file.c: the change to call the __curfile_close() function has a problem with incrementing the data-source's in-use counter. Instead of incrementing the counter in __wt_curfile_open() before calling __curfile_create(), increment it in __curfile_create() after the point where all failure paths close the cursor. cursor/cur_log.c: the change to call the __curlog_close() function means we may be required to clean up a cursor that has not yet acquired the log archive lock. Add the WT_CURLOG_ARCHIVE_LOCK flag to the WT_CURSOR_LOG structure so the close function knows if the log archive lock needs to be released. cursor/cur_log.c: remove the __curlog_reset() call from __curlog_close(), we're closing the cursor, resetting it doesn't do anything useful. cursor/cur_metadata.c: the change to call the __curmetadata_close() function is slightly tricky, be cautious about closing the underlying WT_CURSOR_METADATA.file_cursor field, it may not be set when we call __curmetadata_close(). * comment typo * fix a comment * Add explicit comment on the life-cycle of the WT_CURBACKUP_LOCKER flag. * Review comment, set a specific size on the flags field. Generally, the cursors all use uint8_t for flags, and I don't think it matters much what we use, use the same thing everywhere.
Diffstat (limited to 'src/btree')
-rw-r--r--src/btree/bt_split.c8
1 files changed, 4 insertions, 4 deletions
diff --git a/src/btree/bt_split.c b/src/btree/bt_split.c
index ee9c6ad23e8..ac352b79a25 100644
--- a/src/btree/bt_split.c
+++ b/src/btree/bt_split.c
@@ -1500,7 +1500,7 @@ __split_multi_inmem(
* This code re-creates an in-memory page that is part of a set created
* while evicting a large page, and adds references to any unresolved
* update chains to the new page. We get here due to choosing to keep
- * the results of a split in memory or because and update could not be
+ * the results of a split in memory or because an update could not be
* written when attempting to evict a page.
*
* Clear the disk image and link the page into the passed-in WT_REF to
@@ -2208,9 +2208,9 @@ __wt_split_rewrite(WT_SESSION_IMPL *session, WT_REF *ref)
*
* Build the new page.
*
- * Allocate a WT_REF because the error path uses routines that will ea
- * free memory. The only field we need to set is the record number, as
- * it's used by the search routines.
+ * Allocate a WT_REF, the error path calls routines that free memory.
+ * The only field we need to set is the record number, as it's used by
+ * the search routines.
*/
WT_RET(__wt_calloc_one(session, &new));
new->ref_recno = ref->ref_recno;