JITServerSharedROMClassCache double-initialization after purging all client session data triggers assert

[cjjdespres]

During a purge operation, the JITServer will delete client session data:

openj9/runtime/compiler/runtime/JITClientSession.cpp

Lines 1229 to 1243 in 4af048a

	// Time for a purge operation.
	// Scan the entire table and delete old elements that are not in use
	for (auto iter = _clientSessionMap.begin(); iter != _clientSessionMap.end(); ++iter)
	{
	TR_ASSERT(iter->second->getInUse() >= 0, "_inUse=%d must be positive\n", iter->second->getInUse());
	if (iter->second->getInUse() == 0 &&
	crtTime - iter->second->getTimeOflastAccess() > oldAge)
	{
	if (TR::Options::getVerboseOption(TR_VerboseJITServer))
	TR_VerboseLog::writeLineLocked(TR_Vlog_JITServer, "t=%u Server will purge session data for clientUID %llu of age %lld. Number of clients before purge: %u",
	(uint32_t)_compInfo->getPersistentInfo()->getElapsedTime(), (unsigned long long)iter->first, (long long)oldAge, size());
	ClientSessionData::destroy(iter->second); // delete the client data
	_clientSessionMap.erase(iter); // delete the mapping from the hashtable
	}
	}

It is possible for this operation to delete every entry in the _clientSessionMap. If it does, the next time a client connects, this code will run:

openj9/runtime/compiler/runtime/JITClientSession.cpp

Lines 1122 to 1127 in 4af048a

	// If this is the first client, initialize the shared ROMClass cache
	if (_clientSessionMap.empty())
	{
	if (auto cache = TR::CompilationInfo::get()->getJITServerSharedROMClassCache())
	cache->initialize(jitConfig);
	}

However, the shared ROM class cache will have already been created, leading to an assert triggering in initialize():

openj9/runtime/compiler/runtime/JITServerSharedROMClassCache.cpp

Line 129 in 4af048a

TR_ASSERT(!isInitialized(), "Already initialized");

I think the purge operation may need to check if _clientSessionMap is empty post-purge, and shut down the shared ROM class cache if it is. You can see that the deleteClientSession() function does this properly:

openj9/runtime/compiler/runtime/JITClientSession.cpp

Lines 1172 to 1177 in 4af048a

	// If this was the last client, shutdown the shared ROMClass cache
	if (_clientSessionMap.empty())
	{
	if (auto cache = TR::CompilationInfo::get()->getJITServerSharedROMClassCache())
	cache->shutdown();
	}

[cjjdespres]

Attn @mpirvu. I can put up a fix when I get a chance.

[cjjdespres]

Just to note what we talked about: it's not immediately obvious that we should shut down the shared ROM class cache once the last client disconnects, so maybe the deleteClientSession() should be changed instead. That might introduce a problem with unbounded growth of the shared ROM class cache for long-running JITServer instances, though.

[mpirvu]

The way I am reading the code is the following:

• the purge operation may delete the last attached client and in that case it calls JITServerSharedROMClassCache::shutdown(bool lastClient)
• JITServerSharedROMClassCache::shutdown() deletes the entire ROMClassCache including the persistent allocator used for allocating space for ROMClasses. At the very end it sets _persistentMemory = NULL
• When a new client connects, it checks _clientSessionMap.empty() which is true, and then proceeds to do another cache initialization.
• Inside JITServerSharedROMClassCache::initialize() there is the assert TR_ASSERT(!isInitialized(), "Already initialized"); but this should not trigger because isInitialized() looks at _persistentMemory field which was set to NULL.

[mpirvu]

I think the purge operation may need to check if _clientSessionMap is empty post-purge, and shut down the shared ROM class cache if it is.

I agree with this