Test case for mdb_env_copyfd1 leaking memory and pthread state

copyfd1-leak.c

/*
 * Copyright 2012 Howard Chu, Symas Corp.
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted only as authorized by the OpenLDAP
 * Public License.
 *
 * A copy of this license is available in the file LICENSE in the
 * top-level directory of the distribution or, alternatively, at
 * <http://www.OpenLDAP.org/license.html>.
 */
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include "lmdb.h"

int main(int argc,char * argv[])
{
	int rc;
	int fd;
	MDB_env *env;
	MDB_txn *txn;

	fd = open("temp.bin", O_CREAT|O_RDWR|O_TRUNC, S_IRUSR|S_IWUSR);
	if (fd < 0) {
		fprintf(stderr, "Failed to open temp file: %s\n", strerror(errno));
		return 1;
	}

#define check(rc, x) rc = x; if (rc) { fprintf(stderr, #x ": (%d) %s\n", rc, mdb_strerror(rc)); goto leave; }
	check(rc, mdb_env_create(&env));
	check(rc, mdb_env_set_maxreaders(env, 1));
	check(rc, mdb_env_open(env, "./", 0, 0664));

	// Use only reader slot
	check(rc, mdb_txn_begin(env, NULL, MDB_RDONLY, &txn));

	// Attempt copy
	check(rc, mdb_env_copyfd2(env, fd, MDB_CP_COMPACT));

leave:
	if (txn != NULL) mdb_txn_abort(txn);
	if (env != NULL) mdb_env_close(env);
	return rc;
#undef check
}

copyfd1-leak.patch

diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c
index 79a958b..9532e2f 100644
--- a/libraries/liblmdb/mdb.c
+++ b/libraries/liblmdb/mdb.c
@@ -9904,23 +9904,31 @@ mdb_env_copyfd1(MDB_env *env, HANDLE fd)
 	pthread_t thr;
 	int rc;
 
+	rc = mdb_txn_begin(env, NULL, MDB_RDONLY, &txn);
+	if (rc)
+		return rc;
+
 #ifdef _WIN32
 	my.mc_mutex = CreateMutex(NULL, FALSE, NULL);
 	my.mc_cond = CreateEvent(NULL, FALSE, FALSE, NULL);
 	my.mc_wbuf[0] = _aligned_malloc(MDB_WBUF*2, env->me_os_psize);
-	if (my.mc_wbuf[0] == NULL)
-		return errno;
+	if (my.mc_wbuf[0] == NULL) {
+		rc = errno;
+		goto leave;
+	}
 #else
 	pthread_mutex_init(&my.mc_mutex, NULL);
 	pthread_cond_init(&my.mc_cond, NULL);
 #ifdef HAVE_MEMALIGN
 	my.mc_wbuf[0] = memalign(env->me_os_psize, MDB_WBUF*2);
-	if (my.mc_wbuf[0] == NULL)
-		return errno;
+	if (my.mc_wbuf[0] == NULL) {
+		rc = errno;
+		goto leave;
+	}
 #else
 	rc = posix_memalign((void **)&my.mc_wbuf[0], env->me_os_psize, MDB_WBUF*2);
 	if (rc)
-		return rc;
+		goto leave;
 #endif
 #endif
 	memset(my.mc_wbuf[0], 0, MDB_WBUF*2);
@@ -9937,10 +9945,6 @@ mdb_env_copyfd1(MDB_env *env, HANDLE fd)
 	my.mc_fd = fd;
 	THREAD_CREATE(thr, mdb_env_copythr, &my);
 
-	rc = mdb_txn_begin(env, NULL, MDB_RDONLY, &txn);
-	if (rc)
-		return rc;
-
 	mp = (MDB_page *)my.mc_wbuf[0];
 	memset(mp, 0, NUM_METAS * env->me_psize);
 	mp->mp_pgno = 0;
@@ -9993,6 +9997,8 @@ mdb_env_copyfd1(MDB_env *env, HANDLE fd)
 	while(my.mc_new)
 		pthread_cond_wait(&my.mc_cond, &my.mc_mutex);
 	pthread_mutex_unlock(&my.mc_mutex);
+
+leave:
 	THREAD_FINISH(thr);
 
 	mdb_txn_abort(txn);

valgrind.txt

$ valgrind --leak-check=full ./copyfd1-leak
==5196== Memcheck, a memory error detector
==5196== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5196== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5196== Command: ./copyfd1-leak
==5196==
mdb_env_copyfd2(env, fd, MDB_CP_COMPACT): (-30783) MDB_BAD_RSLOT: Invalid reuse of reader locktable slot
==5196==
==5196== HEAP SUMMARY:
==5196==     in use at exit: 2,119,263 bytes in 188 blocks
==5196==   total heap usage: 280 allocs, 92 frees, 5,284,556 bytes allocated
==5196==
==5196== 2,064 bytes in 1 blocks are possibly lost in loss record 54 of 60
==5196==    at 0x10002717C: malloc_zone_malloc (in /usr/local/Cellar/valgrind/3.11.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==5196==    by 0x100517EFD: _objc_copyClassNamesForImage (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B182: protocols() (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B093: readClass(objc_class*, bool, bool) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x100508C13: gc_init (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10051024E: objc_initializeClassPair_internal(objc_class*, char const*, objc_class*, objc_class*) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10051D132: layout_string_create (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B83C: realizeClass(objc_class*) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B300: copySwiftV1MangledName(char const*, bool) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B2E9: copySwiftV1MangledName(char const*, bool) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B2E9: copySwiftV1MangledName(char const*, bool) (in /usr/lib/libobjc.A.dylib)
==5196==    by 0x10050B2E9: copySwiftV1MangledName(char const*, bool) (in /usr/lib/libobjc.A.dylib)
==5196==
==5196== 2,097,152 bytes in 1 blocks are definitely lost in loss record 60 of 60
==5196==    at 0x100027EEA: malloc_zone_memalign (in /usr/local/Cellar/valgrind/3.11.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==5196==    by 0x100374416: posix_memalign (in /usr/lib/system/libsystem_malloc.dylib)
==5196==    by 0x1000173E5: mdb_env_copyfd1 (mdb.c:9921)
==5196==    by 0x10001732C: mdb_env_copyfd2 (mdb.c:10115)
==5196==    by 0x100000F51: main (copyfd1-leak.c:42)
==5196==
==5196== LEAK SUMMARY:
==5196==    definitely lost: 2,097,152 bytes in 1 blocks
==5196==    indirectly lost: 0 bytes in 0 blocks
==5196==      possibly lost: 2,064 bytes in 1 blocks
==5196==    still reachable: 0 bytes in 0 blocks
==5196==         suppressed: 20,047 bytes in 186 blocks
==5196==
==5196== For counts of detected and suppressed errors, rerun with: -v
==5196== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 14 from 14)