[TSan] Unlock mini-trampolines.c #5597
Conversation
|
@cherusker, |
mono/mini/mini-trampolines.c
Outdated
|
|
||
| static_rgctx_trampolines ++; | ||
| UnlockedIncrement (&static_rgctx_trampolines); /* use the domain lock to synchronise */ |
There was a problem hiding this comment.
that comment is confusing: the domain lock won't protect accessing static_rgtx_trampoline because the domain lock is, well, per domain 🙂 Otherwise, why would you need UnlockedIncrement here?
There was a problem hiding this comment.
The idea was to add some kind of synchronisation by using the domain lock but the comment is false of course; I will remove it! :)
mono/mini/mini-trampolines.c
Outdated
| @@ -1496,10 +1497,9 @@ mono_create_jit_trampoline (MonoDomain *domain, MonoMethod *method, MonoError *e | |||
|
|
|||
| mono_domain_lock (domain); | |||
| g_hash_table_insert (domain_jit_info (domain)->jit_trampoline_hash, method, tramp); | |||
| UnlockedIncrement (&jit_trampolines); /* use the domain lock to synchronise */ | |||
mono/mini/mini-trampolines.c
Outdated
| inited = TRUE; | ||
| } | ||
| num_trampolines++; | ||
| UnlockedIncrement (&rgctx_num_lazy_fetch_trampolines); /* use the trampolines lock to synchronise */ |
There was a problem hiding this comment.
sorry, this comment is wrong (mono_trampolines_lock () is used). However, UnlockedIncrement shouldn't be required here then.
- have all counters in a global place and initialise them in `mono_trampolines_init ()` - use `Unlocked* ()` and `mono_trampolines_lock ()` to mark / synchronise all racy counters
cherusker
force-pushed
the
cherusker-2017-09-16-mini-trampolines-c
branch
from
fb1cf99 to
3cbb352
Compare
|
@lewurm fixed! :) |
|
@monojenkins rebase |
|
cannot rebase:
|
1 similar comment
|
cannot rebase:
|
|
hmmm, in the i386 windows build: I can't imagine it's related, but it looks suspicious. |
|
Its probly not related. |
- have all counters in a global place and initialise them in `mono_trampolines_init ()` - use `Unlocked* ()` and `mono_trampolines_lock ()` to mark / synchronise all racy counters Commit migrated from mono/mono@5d97d9b
guint32 -> gint32: nothing changes since the counters were registered asMONO_COUNTER_INTbefore. Additionally, by usinggint32, existingInterlocked* ()andUnlocked* ()functions can be used.Avoid "function
static" counters and initialise all counters inmono_trampolines_init (): that matches the current (more modern) approach of dealing with counters and it also helps with readability and produces less duplicate code.I think that
Interlocked* ()is no option when calling trampolines which is why I usedUnlocked* ()everywhere; sometimes, however, it can be bundled with other locks (domain and trampolines) to interlock some counters implicitly.