Skip to content

Commit 6ef76c0

Browse files
committed
Only free options objects in Options::shutdown if safe for FrontEnd
Adds a query to TR_FrontEnd (which defaults to "not safe") to answer whether Options::shutdown() should free the command-line options objects. For compilers like JitBuilder and the Test compiler, not freeing the options represents a memory leak and there is not reason not to free the options objects, so these compilers arrange for the objects to be freed. Other FrontEnds (but notably OpenJ9) by default will not free the options objects, because e.g. OpenJ9 cannot guarantee that the options objects will not be later read in one of its many event handlers that continue to operate throughout the shutdown process. This change addresses crashes in OpenJ9 that started happening after we plugged the memory leak represented by the options objects previously not being freed at shutdown. Signed-off-by: Mark Stoodley <[email protected]>
1 parent 91a1729 commit 6ef76c0

File tree

5 files changed

+37
-11
lines changed

5 files changed

+37
-11
lines changed

compiler/control/OMROptions.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2994,19 +2994,22 @@ OMR::Options::shutdown(TR_FrontEnd * fe)
29942994
TR::Options::closeLogsForOtherCompilationThreads(fe);
29952995
}
29962996

2997-
if (TR::Options::getAOTCmdLineOptions()->_countString)
2997+
if (fe->isSafeToFreeOptionsOnShutdown())
29982998
{
2999-
TR::Options::jitPersistentFree(const_cast<void *>(reinterpret_cast<const void *>(TR::Options::getAOTCmdLineOptions()->_countString)));
3000-
}
3001-
TR::Options::jitPersistentFree(_aotCmdLineOptions);
3002-
_aotCmdLineOptions = NULL;
2999+
if (TR::Options::getAOTCmdLineOptions()->_countString)
3000+
{
3001+
TR::Options::jitPersistentFree(const_cast<void *>(reinterpret_cast<const void *>(TR::Options::getAOTCmdLineOptions()->_countString)));
3002+
}
3003+
TR::Options::jitPersistentFree(_aotCmdLineOptions);
3004+
_aotCmdLineOptions = NULL;
30033005

3004-
if (TR::Options::getJITCmdLineOptions()->_countString)
3005-
{
3006-
TR::Options::jitPersistentFree(const_cast<void *>(reinterpret_cast<const void *>(TR::Options::getJITCmdLineOptions()->_countString)));
3006+
if (TR::Options::getJITCmdLineOptions()->_countString)
3007+
{
3008+
TR::Options::jitPersistentFree(const_cast<void *>(reinterpret_cast<const void *>(TR::Options::getJITCmdLineOptions()->_countString)));
3009+
}
3010+
TR::Options::jitPersistentFree(_jitCmdLineOptions);
3011+
_jitCmdLineOptions = NULL;
30073012
}
3008-
TR::Options::jitPersistentFree(_jitCmdLineOptions);
3009-
_jitCmdLineOptions = NULL;
30103013
}
30113014

30123015

compiler/env/FrontEnd.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,3 +318,15 @@ TR_FrontEnd::methodTrampolineLookup(TR::Compilation *comp, TR::SymbolReference *
318318
TR_UNIMPLEMENTED();
319319
return 0;
320320
}
321+
322+
void
323+
TR_FrontEnd::setIsSafeToFreeOptionsOnShutdown(bool isSafe)
324+
{
325+
_isSafeToFreeOptionsOnShutdown = isSafe;
326+
}
327+
328+
bool
329+
TR_FrontEnd::isSafeToFreeOptionsOnShutdown()
330+
{
331+
return _isSafeToFreeOptionsOnShutdown;
332+
}

compiler/env/FrontEnd.hpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ struct TR_BinaryEncodingData
103103
class TR_FrontEnd : private TR::Uncopyable
104104
{
105105
public:
106-
TR_FrontEnd() {}
106+
TR_FrontEnd()
107+
: _isSafeToFreeOptionsOnShutdown(false) // most conservative setting, relied upon by OpenJ9
108+
{}
107109

108110
// --------------------------------------------------------------------------
109111
// Method
@@ -222,6 +224,11 @@ class TR_FrontEnd : private TR::Uncopyable
222224
virtual char *getFormattedName(char *, int32_t, char *, char *, bool);
223225
virtual void printVerboseLogHeader(TR::Options *cmdLineOptions) {}
224226

227+
virtual void setIsSafeToFreeOptionsOnShutdown(bool isSafe=true);
228+
virtual bool isSafeToFreeOptionsOnShutdown();
229+
230+
private:
231+
bool _isSafeToFreeOptionsOnShutdown;
225232
};
226233

227234
#endif

fvtest/compilertest/control/TestJit.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ initializeTestJit(TR_RuntimeHelper *helperIDs, void **helperAddresses, int32_t n
166166

167167
// --------------------------------------------------------------------------
168168
static TR::FrontEnd fe;
169+
fe.setIsSafeToFreeOptionsOnShutdown(true);
170+
169171
auto jitConfig = fe.jitConfig();
170172

171173
initializeAllHelpers(jitConfig, helperIDs, helperAddresses, numHelpers);

jitbuilder/control/Jit.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ initializeJitBuilder(TR_RuntimeHelper *helperIDs, void **helperAddresses, int32_
137137

138138
// --------------------------------------------------------------------------
139139
static TR::FrontEnd fe;
140+
fe.setIsSafeToFreeOptionsOnShutdown(true);
141+
140142
auto jitConfig = fe.jitConfig();
141143

142144
initializeAllHelpers(jitConfig, helperIDs, helperAddresses, numHelpers);

0 commit comments

Comments
 (0)