Page MenuHomePhabricator

Protect Proces::GetMemoryRegionInfo and ::GetFileLoadAddress with a lock
AbandonedPublic

Authored by scott.smith on Apr 26 2017, 3:50 PM.

Details

Reviewers
labath
Group Reviewers
Restricted Project
Summary

Both routines (on Linux, at least) utilize a cache; protect the cache with a mutex to allow concurrent callers.

Diff Detail

Repository
rL LLVM

Event Timeline

scott.smith created this revision.Apr 26 2017, 3:50 PM
labath requested changes to this revision.Apr 27 2017, 1:36 AM
labath added a subscriber: labath.

This is not necessary. NativeProcess classes are only used in lldb-server, which is completely single threaded. If you want to change that, then we should start with a discussion of what you intend to achieve.

This revision now requires changes to proceed.Apr 27 2017, 1:36 AM

This is not necessary. NativeProcess classes are only used in lldb-server, which is completely single threaded. If you want to change that, then we should start with a discussion of what you intend to achieve.

Let me post the other two changes to start a broader discussion. We can center the conversation around whether/how to prime the caches; the other two changes naturally follow from that.

This is not necessary. NativeProcess classes are only used in lldb-server, which is completely single threaded. If you want to change that, then we should start with a discussion of what you intend to achieve.

Let me post the other two changes to start a broader discussion. We can center the conversation around whether/how to prime the caches; the other two changes naturally follow from that.

All your other changes are client-side, so still think this will not matter, but I'll take a look. :)

I meant to respond to the discussion today, but I got sidetracked by the ipv6 thingy. I'll try to look at that tomorrow.

All your other changes are client-side, so still think this will not matter, but I'll take a look. :)

Interesting. Maybe this only matters in the context of unit tests? I was still getting crashes without this change, but now I'm not (I've since git pull'd and switched to another machine). I'll keep investigating.

scott.smith abandoned this revision.Apr 27 2017, 5:17 PM

Further testing shows this is not necessary. I'll abandon it.

Cool, glad that's sorted. I've had some ideas about introducing limited amount of paralellism to the server side, but that's probably is not interesting to you if you're not doing remote debugging.