microprofile icon indicating copy to clipboard operation
microprofile copied to clipboard

Crash in MicroProfileCreateThreadLog() when (S.nNumLogs > MICROPROFILE_MAX_THREADS)

Open MicrosoftComments opened this issue 4 years ago • 0 comments

In this code, it's perfectly possible for pOldest to still be nullptr when it comes to the end (Which I know from observation).

While this condition has an ASSERT, that doesn't stop it immediately trying to reset the null pointer if you're in a release build, with hilarious consequences.

Easy repro case is to set MICROPROFILE_MAX_THREADS to 1 or 2.

This is a pain if you have a variable number of threads being profiled, so there is no safe level to set MICROPROFILE_MAX_THREADS to.

MicroProfileCreateThreadLog()
...
	if(S.nNumLogs == MICROPROFILE_MAX_THREADS && S.nFreeListHead == -1)
	{
		uprintf("recycling thread logs\n");
		//reuse the oldest.
		MicroProfileThreadLog* pOldest = 0;
		uint32_t nIdleFrames = 0;
		for(uint32_t i = 0; i < MICROPROFILE_MAX_THREADS; ++i)
		{
			MicroProfileThreadLog* pLog = S.Pool[i];
			uprintf("tlactive %p, %d.  idle:%d\n", pLog, pLog->nActive, pLog->nIdleFrames); 
			if(pLog->nActive == 2)
			{
				if(pLog->nIdleFrames >= nIdleFrames)
				{
					nIdleFrames = pLog->nIdleFrames;
					pOldest = pLog;
				}
			}
		}
		MP_ASSERT(pOldest);
		MicroProfileLogReset(pOldest);
	}

Possible fix suggestion is to simply fail the function and return nullptr from MicroProfileCreateThreadLog() if it hasn't got any more room. Although this does rather push the problem to the caller.

MicrosoftComments avatar May 14 '20 08:05 MicrosoftComments