WT-2573: free of stack-allocated WT_REF

The error path in __wt_split_rewrite can free a WT_REF that was allocated
on the stack. Allocate the WT_REF instead, rewriting pages in memory isn't
a performance path, and it's a simple solution.

This change means we have never exercised this error path before. This is
an in-memory run which we've recently added to normal stress testing, but
I'm concerned about this, it seems unlikely a page-rewrite would fail in
normal testing.

Also fix a bug found by inspection, we weren't freeing allocated WT_REF
structures that didn't also have associated page images, which could lead
to memory leaks. There are failure cases where we would have allocated a
WT_REF and either not yet attached a page image or, in some cases, there's
no page image to attach. I'm calling the underlying __wt_free_ref() function
to free the WT_REF structure even when there's no associated page: that
looks safe, but regardless, I want to fix that function if this change fails,
not expicitly freeing the WT_REF structure in the split code.

1 files changed, 13 insertions, 11 deletions

diff --git a/src/btree/bt_split.c b/src/btree/bt_split.c
index 341bc0309d9..ec5843d9bf3 100644
--- a/src/btree/bt_split.c
+++ b/src/btree/bt_split.c
@@ -1600,13 +1600,13 @@ __split_multi_inmem_fail(WT_SESSION_IMPL *session, WT_PAGE *orig, WT_REF *ref)
 	/*
 	 * We failed creating new in-memory pages. For error-handling reasons,
 	 * we've left the update chains referenced by both the original and
-	 * new pages. Discard the new pages, setting a flag so the discard code
-	 * doesn't discard the updates on the page.
+	 * new pages. Discard the new allocated WT_REF structures and their
+	 * pages (setting a flag so the discard code doesn't discard the updates
+	 * on the page).
 	 */
-	if (ref->page != NULL) {
+	if (ref->page != NULL)
 		F_SET_ATOMIC(ref->page, WT_PAGE_UPDATE_IGNORE);
-		__wt_free_ref(session, ref, orig->type, true);
-	}
+	__wt_free_ref(session, ref, orig->type, true);
 }
 
 /*
@@ -2164,7 +2164,7 @@ __wt_split_rewrite(WT_SESSION_IMPL *session, WT_REF *ref)
 	WT_DECL_RET;
 	WT_PAGE *page;
 	WT_PAGE_MODIFY *mod;
-	WT_REF new;
+	WT_REF *new;
 
 	page = ref->page;
 	mod = page->modify;
@@ -2180,10 +2180,11 @@ __wt_split_rewrite(WT_SESSION_IMPL *session, WT_REF *ref)
 	 * to re-create a page in memory after it's been reconciled, and that's
 	 * exactly what we want to do.
 	 *
-	 * Build the new page.
+	 * Build the new page. (Allocate a WT_REF because the error path uses
+	 * routines that want to free memory).
 	 */
-	memset(&new, 0, sizeof(new));
-	WT_ERR(__split_multi_inmem(session, page, &new, &mod->mod_multi[0]));
+	WT_RET(__wt_calloc_one(session, &new));
+	WT_ERR(__split_multi_inmem(session, page, new, &mod->mod_multi[0]));
 
 	/*
 	 * The rewrite succeeded, we can no longer fail.
@@ -2203,11 +2204,12 @@ __wt_split_rewrite(WT_SESSION_IMPL *session, WT_REF *ref)
 	__wt_ref_out(session, ref);
 
 	/* Swap the new page into place. */
-	ref->page = new.page;
+	ref->page = new->page;
 	WT_PUBLISH(ref->state, WT_REF_MEM);
 
+	__wt_free(session, new);
 	return (0);
 
-err:	__split_multi_inmem_fail(session, page, &new);
+err:	__split_multi_inmem_fail(session, page, new);
 	return (ret);
 }