From fd785441ffc11969b658625fe66f162589f07bdb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 29 May 2014 13:51:18 -0400 Subject: [PATCH] When using the OSSP UUID library, cache its uuid_t state object. The original coding in contrib/uuid-ossp created and destroyed a uuid_t object (or, in some cases, even two of them) each time it was called. This is not the intended usage: you're supposed to keep the uuid_t object around so that the library can cache its state across uses. (Other UUID libraries seem to keep equivalent state behind-the-scenes in static variables, but OSSP chose differently.) Aside from being quite inefficient, creating a new uuid_t loses knowledge of the previously generated UUID, which in theory could result in duplicate V1-style UUIDs being created on sufficiently fast machines. On at least some platforms, creating a new uuid_t also draws some entropy from /dev/urandom, leaving less for the rest of the system. This seems sufficiently unpleasant to justify back-patching this change. --- contrib/uuid-ossp/uuid-ossp.c | 74 +++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/contrib/uuid-ossp/uuid-ossp.c b/contrib/uuid-ossp/uuid-ossp.c index 71db0ac269..4ecf25bf88 100644 --- a/contrib/uuid-ossp/uuid-ossp.c +++ b/contrib/uuid-ossp/uuid-ossp.c @@ -79,6 +79,42 @@ pguuid_complain(uuid_rc_t rc) errmsg("OSSP uuid library failure: error code %d", rc))); } +/* + * We create a uuid_t object just once per session and re-use it for all + * operations in this module. OSSP UUID caches the system MAC address and + * other state in this object. Reusing the object has a number of benefits: + * saving the cycles needed to fetch the system MAC address over and over, + * reducing the amount of entropy we draw from /dev/urandom, and providing a + * positive guarantee that successive generated V1-style UUIDs don't collide. + * (On a machine fast enough to generate multiple UUIDs per microsecond, + * or whatever the system's wall-clock resolution is, we'd otherwise risk + * collisions whenever random initialization of the uuid_t's clock sequence + * value chanced to produce duplicates.) + * + * However: when we're doing V3 or V5 UUID creation, uuid_make needs two + * uuid_t objects, one holding the namespace UUID and one for the result. + * It's unspecified whether it's safe to use the same uuid_t for both cases, + * so let's cache a second uuid_t for use as the namespace holder object. + */ +static uuid_t * +get_cached_uuid_t(int which) +{ + static uuid_t *cached_uuid[2] = {NULL, NULL}; + + if (cached_uuid[which] == NULL) + { + uuid_rc_t rc; + + rc = uuid_create(&cached_uuid[which]); + if (rc != UUID_RC_OK) + { + cached_uuid[which] = NULL; + pguuid_complain(rc); + } + } + return cached_uuid[which]; +} + static char * uuid_to_string(const uuid_t *uuid) { @@ -109,20 +145,14 @@ string_to_uuid(const char *str, uuid_t *uuid) static Datum special_uuid_value(const char *name) { - uuid_t *uuid; + uuid_t *uuid = get_cached_uuid_t(0); char *str; uuid_rc_t rc; - rc = uuid_create(&uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); rc = uuid_load(uuid, name); if (rc != UUID_RC_OK) pguuid_complain(rc); str = uuid_to_string(uuid); - rc = uuid_destroy(uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); return DirectFunctionCall1(uuid_in, CStringGetDatum(str)); } @@ -166,20 +196,14 @@ uuid_ns_x500(PG_FUNCTION_ARGS) static Datum uuid_generate_internal(int mode, const uuid_t *ns, const char *name) { - uuid_t *uuid; + uuid_t *uuid = get_cached_uuid_t(0); char *str; uuid_rc_t rc; - rc = uuid_create(&uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); rc = uuid_make(uuid, mode, ns, name); if (rc != UUID_RC_OK) pguuid_complain(rc); str = uuid_to_string(uuid); - rc = uuid_destroy(uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); return DirectFunctionCall1(uuid_in, CStringGetDatum(str)); } @@ -202,25 +226,15 @@ uuid_generate_v1mc(PG_FUNCTION_ARGS) static Datum uuid_generate_v35_internal(int mode, pg_uuid_t *ns, text *name) { - uuid_t *ns_uuid; - Datum result; - uuid_rc_t rc; + uuid_t *ns_uuid = get_cached_uuid_t(1); - rc = uuid_create(&ns_uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); - string_to_uuid(DatumGetCString(DirectFunctionCall1(uuid_out, UUIDPGetDatum(ns))), + string_to_uuid(DatumGetCString(DirectFunctionCall1(uuid_out, + UUIDPGetDatum(ns))), ns_uuid); - result = uuid_generate_internal(mode, - ns_uuid, - text_to_cstring(name)); - - rc = uuid_destroy(ns_uuid); - if (rc != UUID_RC_OK) - pguuid_complain(rc); - - return result; + return uuid_generate_internal(mode, + ns_uuid, + text_to_cstring(name)); }