4c20412187794d30583e3b68a102b7a8612d29a1
braney
  Fri Sep 4 13:47:28 2026 -0700
hgTracks: settle a composite subtrack's visibility before the parallel loaders start

The same request could return two different images.  A composite child's
visibility is written lazily, the first time anything asks for it, by
limitedVisFromComposite().  The load phase starts the parallel loader threads
and only waits for them later, and in between the main thread runs
compositeLoad() over every subtrack of the composite, including the ones a
worker is loading right now.  The loop condition there is what performs the
write.  So the main thread wrote the field while a worker read it, and whichever
got there first decided what the track looked like.

findLeavesForParallelLoad() already computes the same value, with the same test,
for exactly the subtracks a worker will load, and then discards it.  Keep it:
write the visibility there, before pthread_create, and skip nothing else.  The
value and the guards are the ones limitedVisFromComposite() uses, so the write
only moves earlier.

Caught on clinvarSubLolly, whose loader divides its 128 pixel default height by
1.5 for pack mode, so the band came out 128 or 85 pixels.  The effect is wider
than one band: on BRCA1_BRCA2_ENIGMA_hg19 at chr17:41196312-41277500 in
Helvetica, the race hid clinvarMain and clinvarSubLolly and collapsed the two
gnomAD variant rows from about 15000 pixels each to 31, an image of 1259 rows
instead of 31967.  barChartTrack.c, chainTrack.c, gtexTracks.c and
halSnakeTrack.c read the same field in their loaders, and three of them use it
to decide what to load rather than how tall to draw.

Verified with the 80 cell pixel sweep from #38094.  Before: 33 of 80 cells
render differently at one thread than at twenty, and the sweep reports
differences when master is compared against itself.  After: 0 of 80, and the
control passes.

refs #38254

diff --git src/hg/hgTracks/hgTracks.c src/hg/hgTracks/hgTracks.c
index abe170effbb..dbda68b224d 100644
--- src/hg/hgTracks/hgTracks.c
+++ src/hg/hgTracks/hgTracks.c
@@ -8130,32 +8130,43 @@
         for (subtrack=track->subtracks; subtrack; subtrack=subtrack->next)
 	    {
 
 	    char *quickLiftFile = cloneString(trackDbSetting(subtrack->tdb, "quickLiftUrl"));
 	    if (doLoadSummary && quickLiftFile)
 	      continue;    
 
 	    if (doLoadSummary && !subtrack->loadSummary)
 	      continue;    
 
 	    if (doLoadSummary && startsWith("bigLolly", subtrack->tdb->type)) 
 	      continue;    
 
 	    if (isTrackForParallelLoad(subtrack))
 		{
-		if (tdbVisLimitedByAncestors(cart,subtrack->tdb,TRUE,TRUE) != tvHide)
-		    {
+		enum trackVisibility subVis = tdbVisLimitedByAncestors(cart,subtrack->tdb,TRUE,TRUE);
+		if (subVis != tvHide)
+		    {
+		    /* Settle this subtrack's visibility here, on the main thread, before
+		     * the worker thread that loads it can read it.  A composite child's
+		     * visibility is otherwise written lazily by limitedVisFromComposite(),
+		     * called from compositeLoad() on the main thread while the worker is
+		     * already running, so the loader could read it either before or after
+		     * the write and the track came out at one of two heights.  This is the
+		     * same value and the same test limitedVisFromComposite() would use, so
+		     * the write only moves earlier.  refs #38254 */
+		    if (tdbIsCompositeChild(subtrack->tdb) && !subtrack->limitedVisSet)
+			subtrack->visibility = subVis;
 		    struct paraFetchData *pfd;
 		    AllocVar(pfd);
 		    pfd->track = subtrack;  // need pointer to be stable
 		    pfd->doLoadSummary = doLoadSummary;
 		    slAddHead(ppfdList, pfd);
 		    subtrack->parallelLoading = TRUE;
 		    }
 		}
 	    }
 	}
     }
 }
 
 static pthread_mutex_t pfdMutex = PTHREAD_MUTEX_INITIALIZER;
 static struct paraFetchData *pfdList = NULL, *pfdRunning = NULL, *pfdDone = NULL, *pfdNeverStarted = NULL;