Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shark: Crash when reading HeapInstance's fields from multiple threads #2084

Closed
DSteve595 opened this issue Mar 11, 2021 · 2 comments · Fixed by #2095
Closed

Shark: Crash when reading HeapInstance's fields from multiple threads #2084

DSteve595 opened this issue Mar 11, 2021 · 2 comments · Fixed by #2095
Milestone

Comments

@DSteve595
Copy link

Description

Given a HeapInstance, calling readFields().any() in two threads simultaneous results in this error:

Exception in thread "Thread-0" java.util.NoSuchElementException: Key 0 is missing in the map.
        at kotlin.collections.MapsKt__MapWithDefaultKt.getOrImplicitDefaultNullable(MapWithDefault.kt:24)
        at kotlin.collections.MapsKt__MapsKt.getValue(Maps.kt:344)
        at shark.internal.ClassFieldsReader.skipStaticFields(ClassFieldsReader.kt:87)
        at shark.internal.ClassFieldsReader.classDumpFields(ClassFieldsReader.kt:55)
        at shark.HprofHeapGraph.classDumpFields$shark_graph(HprofHeapGraph.kt:207)
        at shark.HeapObject$HeapClass.readRecordFields(HeapObject.kt:259)
        at shark.HeapObject$HeapInstance$readFields$1.invoke(HeapObject.kt:450)
        at shark.HeapObject$HeapInstance$readFields$1.invoke(HeapObject.kt:315)
        at kotlin.sequences.FlatteningSequence$iterator$1.ensureItemIterator(Sequences.kt:315)
        at kotlin.sequences.FlatteningSequence$iterator$1.hasNext(Sequences.kt:303)
        at kotlin.sequences.SequencesKt___SequencesKt.any(_Sequences.kt:1179)

Steps to Reproduce

Sample project.

Run ./gradlew run to repro.

Expected behavior: [What you expect to happen]

Version Information

Shark 2.6.

@pyricau
Copy link
Member

pyricau commented Mar 24, 2021

Thanks! We definitely should look at making LeakCanary thread safe yet efficient. Here the fix might be to make readFields() leverage a thread local.

@pyricau
Copy link
Member

pyricau commented Mar 25, 2021

ClassFieldsReader holds the metadata for all class fields as one giant immutable byte array, so that's thread safe. However, it has a var position that is reset every time we read fields from one class and then moves forward as we read. We need to make that thread safe.

pyricau added a commit that referenced this issue Mar 25, 2021
This allows position to be thread local.

Fixes #2084
@pyricau pyricau added this to the 2.7 milestone Mar 25, 2021
ghost pushed a commit to shivagowda/leakcanary that referenced this issue Nov 8, 2021
This allows position to be thread local.

Fixes square#2084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants