Fix trivial leak in ThreadFilter

[jbachorik]

What does this PR do?:
It fixes a trivial leak when the destructor of ThreadFilter class did not release the memory allocated for _bitmaps.

Motivation:
Mostly just a code cleanup - with our current usage the ThreadFilter instance will remain active during the whole lifespan of the profiled application so the memory would not really 'leak'. But it is better to have a correct destructor if the usage of ThreadFilter would change in the future.

Additional Notes:

How to test the change?:

For Datadog employees:

• [] If this PR touches code that signs or publishes builds or packages, or handles
  credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
• This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

[github-actions]

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:	runner@fv-az1776-414
Working Directory:	/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:	make -j4 clean all
Clang Version:	Ubuntu clang version 14.0.0-1ubuntu1.1
Date:	Tue Sep 17 13:20:27 2024

Bug Summary

Bug Type	Quantity	Display?
All Bugs	5
Logic error
Assigned value is garbage or undefined	1
Dereference of null pointer	3
Unused code
Dead nested assignment	1

Reports

Bug Group	Bug Type ▾	File	Function/Method	Line	Path Length
Logic error	Assigned value is garbage or undefined	dwarf.cpp	parseInstructions	244	20
Unused code	Dead nested assignment	vmStructs.cpp	checkNativeBinding	879	1
Logic error	Dereference of null pointer	safeAccess.h	load	32	18
Logic error	Dereference of null pointer	symbols_linux.h	ElfParser	129	32
Logic error	Dereference of null pointer	flightRecorder.cpp	flush	1504	8

[github-actions]

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (5)

• Obsolete function 'alloca' called.
• Member variable 'ScopeDesc::_stream' is not initialized in the constructor.
• Member variable 'ScopeDesc::_method_offset' is not initialized in the constructor.
• Member variable 'ScopeDesc::_bci' is not initialized in the constructor.
• Member variable 'ElfParser::_vaddr_diff' is not initialized in the constructor.

Style Violations (159)

• Class 'AsyncSampleMutex' has a constructor with 1 argument that is not explicit.
• Local variable 'jni' shadows outer function
• Local variable 'size' shadows outer function
• Local variable 'prev' shadows outer function
• The class 'NativeFunc' does not declare a constructor although it has private member variables which likely require initialization.
• Local variable 'count' shadows outer function
• Local variable 'i' shadows outer variable
• Consider using std::transform algorithm instead of a raw loop.
• struct member 'Tag::value' is never used.
• struct member 'ContextPage::capacity' is never used.
• struct member 'ContextPage::storage' is never used.
• The function 'check' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• Class 'Dictionary' has a constructor with 1 argument that is not explicit.
• struct member 'DictRow::keys' is never used.
• struct member 'DictRow::next' is never used.
• Local variable 'pc_off' shadows outer variable
• Class 'WallClockEpochEvent' has a constructor with 1 argument that is not explicit.
• struct member 'QueueTimeEvent::_start' is never used.
• struct member 'QueueTimeEvent::_end' is never used.
• struct member 'QueueTimeEvent::_task' is never used.
• struct member 'QueueTimeEvent::_scheduler' is never used.
• struct member 'QueueTimeEvent::_origin' is never used.
• The class 'VirtualSpaceSummary' does not declare a constructor although it has private member variables which likely require initialization.
• The class 'GCHeapSummary' does not declare a constructor although it has private member variables which likely require initialization.
• The class 'CollectedHeap' does not declare a constructor although it has private member variables which likely require initialization.
• Class 'Element' has a constructor with 1 argument that is not explicit.
• Class 'NoField' has a constructor with 1 argument that is not explicit.
• Class 'MutexLocker' has a constructor with 1 argument that is not explicit.
• Class 'MethodList' has a constructor with 1 argument that is not explicit.
• Class 'ScopeDesc' has a constructor with 1 argument that is not explicit.
• The function 'is_nofield' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'check' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'stop' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'interval' overrides a function in a base class but is not marked with a 'override' specifier.
• Condition 'enabled()' is always false
• Condition 'usage._maxSize==-1' is always false
• The scope of the variable 'class_name_id' can be reduced.
• The scope of the variable 'method_name_id' can be reduced.
• The scope of the variable 'method_sig_id' can be reduced.
• Variable 'class_name_id' is assigned a value that is never used.
• Variable 'method_name_id' is assigned a value that is never used.
• Variable 'method_sig_id' is assigned a value that is never used.
• struct member 'CpuTime::real' is never used.
• struct member 'CpuTime::user' is never used.
• struct member 'CpuTime::system' is never used.
• The function 'name' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'interval' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'check' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'stop' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'enableEvents' overrides a function in a base class but is not marked with a 'override' specifier.
• struct member 'jvmtiStackInfoExtended::frame_buffer' is never used.
• The class 'J9WallClock' does not declare a constructor although it has private member variables which likely require initialization.
• The function 'name' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'interval' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'stop' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'enableEvents' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'name' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'interval' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'stop' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'enableEvents' overrides a function in a base class but is not marked with a 'override' specifier.
• Unused variable: kept_classes
• Condition 'level>=_level' is always true
• Condition 'frames_size>0' is always true
• The function 'rewind' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'next' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'size' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'check' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'start' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'enableEvents' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'registerThread' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'unregisterThread' overrides a function in a base class but is not marked with a 'override' specifier.
• The function 'interval' overrides a function in a base class but is not marked with a 'override' specifier.
• Variable 'error' is assigned a value that is never used.
• Variable 'error' is assigned a value that is never used.
• Variable 'error' is assigned a value that is never used.
• Variable 'error' is assigned a value that is never used.
• Unused private function: 'StackFrame::withinCurrentStack'
• The comparison 'EMPTY_FRAME_SIZE > 0' is always false.
• The comparison 'EMPTY_FRAME_SIZE > 0' is always false.
• The scope of the variable 'prev_sp' can be reduced.
• The comparison 'EMPTY_FRAME_SIZE > 0' is always false.
• The comparison 'EMPTY_FRAME_SIZE > 0' is always false.
• Class 'SymbolDesc' has a constructor with 1 argument that is not explicit.
• Class 'MemoryMapDesc' has a constructor with 1 argument that is not explicit.
• Local variable 'flag_addr' shadows outer variable
• struct member 'ASGCT_CallTrace::env' is never used.
• struct member 'ASGCT_CallTrace::frames' is never used.
• struct member 'JVMTIFunctions::unused1' is never used.
• struct member 'JVMTIFunctions::unused2' is never used.
• The scope of the variable 'delayedCounter' can be reduced.
• Variable 'error' is assigned a value that is never used.
• The function 'Agent_OnLoad' is never used.
• The function 'GetJ9vmThread' is never used.
• The function 'JNI_OnLoad' is never used.
• The function 'JNI_OnUnload' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_currentTicks0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_describeDebugCounters0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_dump0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_execute0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_filterThread0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getContextPage0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getContextPageOffset0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getDebugCounters0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getMaxContextPages0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getSamples' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_getTid0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_mallocArenaMax0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_recordSettingEvent0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_recordTrace0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_registerConstant0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_stop0' is never used.
• The function 'Java_com_datadoghq_profiler_JavaProfiler_tscFrequency0' is never used.
• The function '_min' is never used.
• The function 'accept' is never used.
• The function 'appendRecording' is never used.
• The function 'arg0' is never used.
• The function 'arg2' is never used.
• The function 'assign' is never used.
• The function 'body' is never used.
• The function 'clearParsingCaches' is never used.
• The function 'compiledMethod' is never used.
• The function 'copyTo' is never used.
• The function 'covers' is never used.
• The function 'cpuMonitorCycle' is never used.
• The function 'cputime' is never used.
• The function 'deallocateLineNumberTable' is never used.
• The function 'fileSize' is never used.
• The function 'flushJfr' is never used.
• The function 'fromHandle' is never used.
• The function 'fromMethodID' is never used.
• The function 'getCounter' is never used.
• The function 'getJavaTraceInternal' is never used.
• The function 'getJavaTraceJvmti' is never used.
• The function 'getThreadId' is never used.
• The function 'hasClassNames' is never used.
• The function 'hasCompilerStructs' is never used.
• The function 'inJava' is never used.
• The function 'install' is never used.
• The function 'isEntryFrame' is never used.
• The function 'is_readable_pointer' is never used.
• The function 'jarg0' is never used.
• The function 'lastJavaFP' is never used.
• The function 'notify' is never used.
• The function 'ntoh64' is never used.
• The function 'patchImport' is never used.
• The function 'restart' is never used.
• The function 'schedPolicy' is never used.
• The function 'supported' is never used.
• The function 'uninstall' is never used.
• The function 'units' is never used.
• The function 'updateCounter' is never used.
• The function 'waitUntil' is never used.
• The function 'writeListSetting' is never used.

[richardstartin]

no functional impact but keeps tsan happy?

[jbachorik]

This was actually not pointed out by tsan - and we are not running it by default anyway.
I came to this while trying to rule out a sneaky memory leak in profiler before blaming malloc fragmentation again,