gh-jbevain/cecil
Watch
1
0
Fork
You've already forked cecil
 0
Code Issues 79 Pull requests 35 Releases 6 Wiki Activity

Is this locking correct in ModuleDefinition? #951

New issue
Open
opened 2024-06-19 17:54:25 -05:00 by omajid · 1 comment
omajid commented 2024-06-19 17:54:25 -05:00 (Migrated from github.com)

github.com/jbevain/cecil@8e1ae7b4ea/Mono.Cecil/ModuleDefinition.cs (L387-L393)

It seems like this sequence of events might be possible, resulting in creation of an DefaultAssemblyResolver instance that's never disposed:

  1. Thread 1 sees assembly_resolver.value == null
  2. Thread 2 sees assembly_resolver.value == null
  3. Thread 1 enters the lock block and sets assembly_resolver to a new instance
  4. Thread 2 enters the lock block and re-sets assembly_resolver to a new instance

The instance in step 3 is now lost and was never disposed.

https://github.com/jbevain/cecil/blob/8e1ae7b4ea67ccc38cb8db3ded6802643109ffd7/Mono.Cecil/ModuleDefinition.cs#L387-L393 It seems like this sequence of events might be possible, resulting in creation of an DefaultAssemblyResolver instance that's never disposed: 1. Thread 1 sees `assembly_resolver.value == null` 2. Thread 2 sees `assembly_resolver.value == null` 3. Thread 1 enters the `lock` block and sets `assembly_resolver` to a new instance 4. Thread 2 enters the `lock` block and re-sets `assembly_resolver` to a new instance The instance in step 3 is now lost and was never disposed.
ltrzesniewski commented 2024-06-20 10:07:14 -05:00 (Migrated from github.com)

I agree, using if/lock/if would be appropriate as it's a standard pattern, or just changing = to ??= in this case.

I agree, using `if`/`lock`/`if` would be appropriate as it's a standard pattern, or just changing `=` to `??=` in this case.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
archive/20260925T204515Z-rescue/michaelglass/fix-portable-pdb-end-sequence-point
archive/20260925T204515Z-rescue/medhatiwari/fix-compressed-int32-be-jbevain
archive/20260925T204515Z-rescue/jonpryor/dev/jonpryor/jonp-more-ByteBuffer-tests
archive/20260925T204515Z-rescue/sbomer/cecil-sourcelink-fix-jbevain
archive/20260925T204515Z-rescue/Alexx999/pr/pdb-native-off-by-one
archive/20260925T204515Z-rescue/Unity-Technologies/circular-type-thread-safe
archive/20260925T204515Z-rescue/Unity-Technologies/proper-param0xffff-fix
archive/20260925T204515Z-rescue/ColinTimBarndt/fix/import-reference
archive/20260925T204515Z-rescue/tags/0.11.5-patch-6af2136
archive/20260925T204515Z-rescue/Unity-Technologies/avoid-unnecessary-import
archive/20260925T204515Z-rescue/Zastai/add-allows-ref-struct-constraint
archive/20260925T204515Z-rescue/omajid/dispose-resources-nativepdbwriter
byreflike-constraint
archive/20260925T204515Z-rescue/windyuuy/feature/fixbugs
archive/20260925T204515Z-rescue/denis-kudelin/transitive-dependency-fix
param0xffff
archive/20260925T204515Z-rescue/kekyo/fix-maxstack-ignored
archive/20260925T204515Z-rescue/SteveGilham/develop/issue-863
archive/20260925T204515Z-rescue/tom-englert/#859
archive/20260925T204515Z-rescue/Zastai/fix-sequence-point-order
archive/20260925T204515Z-rescue/marek-safar/instruction
archive/20260925T204515Z-rescue/kant2002/kant/import-netmodule
archive/20260925T204515Z-rescue/KirillOsenkov/dev/kirillo/embed
stream-file-name
archive/20260925T204515Z-rescue/simonferquel/add-support-for-filename-info-in-custom-streams
archive/20260925T204515Z-rescue/teo-tsirpanis/const-type-simplify
archive/20260925T204515Z-rescue/teo-tsirpanis/netfx-assembly-resolver
unsafe
get-type-remove-cache
pdbchecksum
release/0.10
archive/20260925T204515Z-rescue/SteveGilham/develop/issue-502
archive/20260925T204515Z-rescue/atykhyy/short-insn-form-checks
archive/20260925T204515Z-rescue/tmat/DiaSymReader
archive/20260925T204515Z-rescue/atykhyy/metadata-comparer
release/0.9
archive/20260925T204515Z-rescue/gluck/typeref_cache
archive/20260925T204515Z-rescue/poizan42/master
archive/20260925T204515Z-rescue/tags/unity3.0
0.11.6
0.11.5
0.11.4
0.11.3
0.11.2
0.11.1
0.11
0.10.4
0.10.3
0.10.2
0.10.1
0.10
0.10-beta7
0.10-beta6
0.10-beta5
0.10-beta4
0.10-beta3
0.10-beta2
0.10-beta1
0.9.6
0.9.5
0.9.4
0.9.3
0.9.2
0.9.1
0.9
Labels
Clear labels   
1.0

   
bug

   
enhancement

   
invalid
No labels
1.0
bug
enhancement
invalid
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-jbevain/cecil#951
No description provided.