Prevent race condition in LivenessTracker

[jbachorik]

What does this PR do?:
Prevents a race condition leading to crash when a partially filled up tracking entry is accessed

Motivation:
Currently, the liveness tracking table is flushed under a shared lock. This allows for tracking part to add new elements in parallel, as they are run under shared lock as well. Most of the time this works fine, but if a table is being flushed and an entry is just partially filled in, with the ref set to non-null but frames still not set, this will lead to SIGSEG.

Additional Notes:
In addition to the change of locking mode, this PR contains a bunch of extra asserts and trivial changes to make the code more expressive and consistent.

Fixes #87

How to test the change?:
I was using the following command to provoke the crash

java -javaagent:dd-java-agent.jar -Ddd.profiling.enabled=true -Ddd.profiling.upload.period=5 -Ddd.profiling.start-force-first=true -Ddd.profiling.ddprof.liveheap.enabled=true -Xmx290m -jar renaissance-gpl-0.14.2.jar akka-uct -r 2000

The crashed followed within 15 minutes without this patch. With this patch I don't observe the crash after more than 4 hours of runtime.

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.
• JIRA: SCP-425

Unsure? Have a question? Request a review!

[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 (162)

• 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 'ObjectSampler' does not declare a constructor although it has private member variables which likely require initialization.
• 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.
• 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.
• 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.
• Variable 'jvmti' is assigned a value that is never used.
• Unused variable: kept_classes
• Condition 'level>=_level' 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.
• 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.
• Variable 'ctx' can be declared with const
• The function 'Agent_OnAttach' 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_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 'VMObjectAlloc' is never used.
• The function '_min' 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 '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 'java_update_version' 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 'restartProfiler' 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.

[github-actions]

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:	runner@fv-az979-609
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:	Fri Apr 26 17:07:48 2024

Bug Summary

Bug Type	Quantity	Display?
All Bugs	6
Logic error
Assigned value is garbage or undefined	1
Dereference of null pointer	2
Result of operation is garbage or undefined	1
Unused code
Dead initialization	1
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	235	20
Unused code	Dead initialization	livenessTracker.cpp	cleanup_table	45	1
Unused code	Dead nested assignment	vmStructs.cpp	checkNativeBinding	818	1
Logic error	Dereference of null pointer	flightRecorder.cpp	flush	1410	8
Logic error	Dereference of null pointer	safeAccess.h	load	35	18
Logic error	Result of operation is garbage or undefined	dwarf.h	getSLeb	142	25