304e9ab6e3a62d7863b144478fc1ac621623a617 braney Wed Jul 29 11:42:01 2026 -0700 Fix data race on the chromAlias caches that was crashing hgTracks in parallel decorator loads. chromAliasFindNative() and chromAliasFindAliases() lazily created their static cache hashes and did their first lookup outside the mutex, taking the lock only for the miss path. hgTracks loads decorators on real pthreads, so two threads arriving before a cache existed could race on it and SIGSEGV in hashLookup. Move the lock above the lazy init and the first lookup so the whole cache access is inside it. Both paths of chromAliasFindNative() now return an allocated string rather than the cache-owned pointer, so document that the caller owns the result, free it in decorationNativeItem() (called once per decoration), and drop the redundant outer cloneString() in hgOfficialChromName(). refs #37955 diff --git src/hg/lib/chromAlias.c src/hg/lib/chromAlias.c index a95a91b42d2..5d54b603386 100644 --- src/hg/lib/chromAlias.c +++ src/hg/lib/chromAlias.c @@ -403,37 +403,37 @@ char *findNativeHashes(char *alias) /* Find a native sequence given an alias using the hash tables. */ { struct chromAlias *chromAlias = (struct chromAlias *)hashFindVal(chromAliasGlobals.aliasToChromHash, alias); if (chromAlias != NULL) return cloneString(chromAlias->chrom); return NULL; } char *chromAliasFindNative(char *alias) /* Find the native seqName for a given alias. */ { static struct hash *cachedNative; char *chrom; +/* Everything below touches the shared cache, so it all has to be under the + * lock. hgTracks calls this from several threads at once (loading decorators, + * for example) and an unlocked read or lazy init here will corrupt the hash. */ +getLock(); if (cachedNative == NULL) cachedNative = newHash(6); -if ((chrom = hashFindVal(cachedNative, alias)) != NULL) - return chrom; - -getLock(); if ((chrom = hashFindVal(cachedNative, alias)) == NULL) { if (chromAliasGlobals.bbi) chrom = bbiAliasFindNative(chromAliasGlobals.bbi, chromAliasGlobals.bptList, chromAliasGlobals.lm, alias); else if (chromAliasGlobals.aliasToChromHash) chrom = findNativeHashes(alias); hashAdd(cachedNative, alias, cloneString(chrom)); } releaseLock(); return cloneString(chrom); } struct slName *findAliasesHashes(char *seqName) @@ -446,37 +446,35 @@ { struct chromAlias *chromAlias = (struct chromAlias *)thisEl->val; struct slName *name = newSlName(chromAlias->alias); slAddHead(&slList, name); } return slList; } struct slName *chromAliasFindAliases(char *seqName) /* Find the aliases for a given seqName. */ { static struct hash *cachedAliases; struct slName *aliases; +/* As in chromAliasFindNative, the whole cache access has to be inside the lock. */ +getLock(); if (cachedAliases == NULL) cachedAliases = newHash(6); -if ((aliases = hashFindVal(cachedAliases, seqName)) != NULL) - return aliases; - -getLock(); if ((aliases = hashFindVal(cachedAliases, seqName)) == NULL) { if (chromAliasGlobals.bbi) aliases = bbiAliasFindAliases(chromAliasGlobals.bbi,chromAliasGlobals.lm, seqName); else if (chromAliasGlobals.chromToAliasHash) aliases = findAliasesHashes(seqName); hashAdd(cachedAliases, seqName, aliases); } releaseLock(); return aliases; } char *chromAliasFindSingleAlias(char *seqName, char *authority)