MonetDB: Feb2013 - Properly realloc the namespace

Stefan Manegold Stefan.Manegold at cwi.nl
Wed Dec 19 16:34:39 CET 2012


Martin,

assertions are not the most suitable way to check for malloc failures.

Assertions are to *assert* facts that must hold, i.e., the would be a
logical error if they do not hold.  Consequently, assertions are only
enabled during development and testing, but disabled (for among others for
performance reasons) in any production builds.  If triggered, assertions
stop the entire process instantly.

Failing mallocs is a runtime exception that can occur at any time for
various reasons. The code should check for this and gracefully handle such
situations with proper exeption handling.

Having said that, in particular is the man namespace is local per
query/client, a failure in one query/client should not crash the whole
server by an assertion (during development/testing) or a segfault (in
production builds), but rather only greacefull trigger and handle an
exception for that one query/client.
Also, if namespaces are global and a malloc failure prevent the whole server
from continuing properly, it should at least (try to) gracefully shut itself
down (with a proper error message), rather than crashing instantly with an
assertion of segfault.

Best,
Stefan

On Wed, Dec 19, 2012 at 11:53:37AM +0100, Martin Kersten wrote:
> Changeset: 2a9bd075c039 for MonetDB
> URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=2a9bd075c039
> Modified Files:
> 	monetdb5/mal/mal_namespace.c
> Branch: Feb2013
> Log Message:
> 
> Properly realloc the namespace
> The expandName always lead to a different anchoring point of the namespace.
> This may leave concurrent threads with dead meat.
> This patch reduces the chance on a SEGfault.
> 
> 
> diffs (37 lines):
> 
> diff --git a/monetdb5/mal/mal_namespace.c b/monetdb5/mal/mal_namespace.c
> --- a/monetdb5/mal/mal_namespace.c
> +++ b/monetdb5/mal/mal_namespace.c
> @@ -66,26 +66,13 @@ typedef struct NAMESPACE{
>  static Namespace namespace;
>  
>  static void expandNamespace(void){
> -	str *nme;
> -	size_t *length;
> -	int *link, newsize, incr = 2048;
> -
> -	newsize= namespace.size + incr;
> -	nme= (str *) GDKmalloc(sizeof(str *) * newsize);
> -	assert(nme != NULL); /* we cannot continue */
> -	link= (int *) GDKmalloc(sizeof(int) * newsize);
> -	assert(link != NULL); /* we cannot continue */
> -	length = (size_t *) GDKmalloc(sizeof(size_t) * newsize);
> -	assert(length != NULL); /* we cannot continue */
> -
> -	memcpy(nme, namespace.nme, sizeof(str *) * namespace.nmetop);
> -	memcpy(link, namespace.link, sizeof(int) * namespace.nmetop);
> -	memcpy(length, namespace.length, sizeof(size_t) * namespace.nmetop);
> -
> -	namespace.size += incr;
> -	GDKfree(namespace.nme); namespace.nme= nme;
> -	GDKfree(namespace.link); namespace.link= link;
> -	GDKfree(namespace.length); namespace.length= length;
> +	namespace.size += 2048;
> +	namespace.nme= (str *) GDKrealloc(namespace.nme, sizeof(str *) * namespace.size);
> +	assert(namespace.nme != NULL); /* we cannot continue */
> +	namespace.link= (int *) GDKrealloc(namespace.link, sizeof(int) * namespace.size);
> +	assert(namespace.link != NULL); /* we cannot continue */
> +	namespace.length = (size_t *) GDKrealloc(namespace.length, sizeof(size_t) * namespace.size);
> +	assert(namespace.length != NULL); /* we cannot continue */
>  }
>  
>  void initNamespace(void) {
> _______________________________________________
> checkin-list mailing list
> checkin-list at monetdb.org
> http://mail.monetdb.org/mailman/listinfo/checkin-list
> 

-- 
| Stefan.Manegold @ CWI.nl | DB Architectures (INS1) |
| http://CWI.nl/~manegold/ | Science Park 123 (L321) |
| Tel.: +31 (0)20 592-4212 | 1098 XG Amsterdam  (NL) |
_______________________________________________
developers-list mailing list
developers-list at monetdb.org
http://mail.monetdb.org/mailman/listinfo/developers-list



More information about the developers-list mailing list