Use file locks to synchronize access to ModuleCache.

[ovyalov]

Summary

Concurrent remote test runs may attempt to read and write from module cache simultaneously.
This CL intends to fix such races by using file-based locks - module id is used as a lock file name providing synchronization on per-module basis.

[zturner]

Would a mutex not work? At least on Windows we can have mutexes that can synchronize among different processes. LockFile will lock a file for exclusive access by a process, but if another process fails to acquire the lock, we won't be able to distinguish the failure from an a different kind of error, like an I/O error.

Also I've never really liked the concept of lock files to begin with, because it requires a cleanup step which won't happen if the program crashes. If there's a way to make this work with a true synchronization primitive I think that would be better. On Windows you do this by just calling CreateMutex and passing a non-null value for the name. Not sure on other platforms.

[ovyalov]

In D9056#157199, @zturner wrote:

Would a mutex not work? At least on Windows we can have mutexes that can synchronize among different processes. LockFile will lock a file for exclusive access by a process, but if another process fails to acquire the lock, we won't be able to distinguish the failure from an a different kind of error, like an I/O error.

Also I've never really liked the concept of lock files to begin with, because it requires a cleanup step which won't happen if the program crashes. If there's a way to make this work with a true synchronization primitive I think that would be better. On Windows you do this by just calling CreateMutex and passing a non-null value for the name. Not sure on other platforms.

Thanks for suggestion - mutex sounds like a good alternative. On the other hand, AFAIK file locks are capable to handle program crash properly and to revoke ownership from terminated process.
There are POSIX named semaphores which might be used as system-wide mutexes but I don't know for sure how widely they are supported on all platforms - file-based locks seems to be pretty widely adopted.

If you feel strongly against using file locks on Windows I can rewrite CL with mutexes - otherwise, we can do it iteratively:

1. Make initial version of this CL based on file locks to enable concurrent test runs.
2. I will migrate solution to use mutex soon (within a month) .

[zturner]

If it's able to handle process crashing without leaving lock files around, maybe it's fine as is. I've added some additional suggestions. I think there's still one more issue that stands out, which is that it's probably better to use the existing file hadnle that is already contained in the Module rather than calling CreateFile(..OPEN_EXISTING..) but I think that will be difficult to fix, as it's not easy to access the HANDLE being used internally by the Module. I don't think it will be broken as is, so I'm mostly just adding this as a note. Eventually one of us on the Windows side can fix this when it becomes an issue (although you're more than welcome to try if you want). I think it's the DataBuffer in the ObjectFile that has a file handle open already, because it's mmap'ed part of the object file. So the best fix would be to somehow get that HANDLE that was used to create the memory map, and use that for the call to LockFileEx.

source/Host/windows/LockFileWindows.cpp

27–29

You can get by without creating an event handle at all if you use GetOverlappedResult() instead of WaitForSingleObject. I slightly prefer that to using the event handle, especially if this is going to be happening very frequently. If you decide to do that, just leave the hEvent initialized to 0 and change the WaitForSingleObject to GetOverlappedResult.

53

Remove this call to _close, as as m_file should refer to the original module file, not a separate "lock file" (see later where I elaborate on this).

91–93

Same as before, we can remove the event handle and just use GetOverlappedResult

source/Utility/ModuleCache.cpp

142–145

Per the previous comments, can move all of this code into LockFilePosix.

source/Utility/ModuleCache.h

75–76

So the way locking a file works on windows, is that if you want to lock "foo.exe", you get a handle to foo.exe and you lock that. You don't make ".lock/<lock file name>" and then lock that, which is what it looks like you're doing. Because if you do that, you've got a totally new file, and then the ranges that you're passing to LockFileEx and UnlockFileEx don't make sense in the context of this new file.

Does this work on Linux? How does it know that even though your fd refers to .lock/<lock file name>, the range you're specifying refers to the original file?

In any case, maybe the best thing to do is have all this logic about building up the lock file name be internal to the LockFilePosix / LockFileWindows implementation. Somehow you will have to pass a HANDLE to the actual module file. If there's already a HANDLE deep down inside of the ModuleSP somewhere, best to use that. Otherwise you can probably just call CreateFile(... OPEN_EXISTING ...) on the module spec. But all of this can be internal to the FileLock class. Just have a constructor that takes a FileSpec to the thing you want to lock. On the linux side do the subdirectory thing, and on the Windows side just call CreateFile() on that path directly.

[zturner]

source/Host/windows/LockFileWindows.cpp

53

Sorry, a little clarification on this. If you call CreateFile(...OPEN_EXISTING...) then change this to CloseHandle(). If you end up figuring out a way to get the existing handle that the Module is using, then don't call CloseHandle when you release the lock.

[clayborg]

This doesn't seem to take care of the case:

process 1: calls ModuleCache::GetAndPut() on line 134 and then gets a path to the file lock file and creates and locks it
process 1: Starts getting the file and writing it to disk by creating it and writes a half of the file's bytes to the cache location "/tmp/usr/lib/libfoo.so"
process 2: calls ModuleCache::GetAndPut() on line 134 and notices there is a "/tmp/usr/lib/libfoo.so" in the cache and it returns the incomplete copy of the file

Don't we need to take the lock before the first call to "auto error = Get (root_dir_spec" on line 134?

If this is an issue we could read to a temp file first ("/tmp/usr/lib/libfoo.so.<UUID>") and then move it back to "/tmp/usr/lib/libfoo.so" atomically after we are done writing the file contents, and then unlock the file lock. This would prevent people from getting an incomplete copy in process 2?

[ovyalov]

In D9056#157301, @clayborg wrote:

This doesn't seem to take care of the case:

process 1: calls ModuleCache::GetAndPut() on line 134 and then gets a path to the file lock file and creates and locks it
process 1: Starts getting the file and writing it to disk by creating it and writes a half of the file's bytes to the cache location "/tmp/usr/lib/libfoo.so"
process 2: calls ModuleCache::GetAndPut() on line 134 and notices there is a "/tmp/usr/lib/libfoo.so" in the cache and it returns the incomplete copy of the file

Don't we need to take the lock before the first call to "auto error = Get (root_dir_spec" on line 134?

If this is an issue we could read to a temp file first ("/tmp/usr/lib/libfoo.so.<UUID>") and then move it back to "/tmp/usr/lib/libfoo.so" atomically after we are done writing the file contents, and then unlock the file lock. This would prevent people from getting an incomplete copy in process 2?

Thanks for catching this - I missed some race conditions:(
Since there is additional race maybe introduced by lock file removal (when one process is about to delete lock file and another process is going to open it) I decided to follow Zach advise and use mutexes.

[ovyalov]

1. Converted solution to use named mutexes instead of file locks.
2. Fixed race condition in GetAndPut by moving lock up to the beginning of the method.

[zturner]

include/lldb/Host/Mutex.h

36 ↗

(On Diff #23940)

I think we should change from returning int to either returning or taking by reference an lldb_private::Error. The same applies for the rest of the functions.

The same applies for all the rest of the functions. I know you are just copying the old API format, but maybe if it's not too much additional churn this would be a good time to convert this API to use the Error class.

50 ↗

(On Diff #23940)

If we change from int to lldb_private::Error, then this failure_message can go away. It's kind of a bad idea to have default parameters on virtual methods anwyay, because depending on who derives from it, you might get different default values depending on what type of pointer you call through, which is confusing.

260–261 ↗

(On Diff #23940)

Just curious, what is the purpose of this Global prefix? Windows doesn't require it, does posix require this "/" at the beginning? It would be nice if the names were at least somewhat consistent. Also it would be good if the mutexes could be "namespaced" so to speak, so that if you run some shell command or tool to display all the mutexes on the system, we can easily identify an LLDB mutex from some other mutex. With that in mind, what about a template like "lldb-$pid-$mutex_name

source/Host/windows/Mutex.cpp

118–121 ↗

(On Diff #23940)

HANDLE is already a typedef of void*, is this necessary?

137 ↗

(On Diff #23940)

Might as well use nullptr instead of NULL for consistency.

[ovyalov]

include/lldb/Host/Mutex.h

36 ↗

(On Diff #23940)

I support this idea of using Error instead of int but it seems too many things will be involved in change - ModuleList, Debugger, ConnectionFileDescriptor, GDBRemote* classes,...
Let's make it as a separate redesign CL.

260–261 ↗

(On Diff #23940)

Global means that mutex will be visible within all user's Terminal sessions - it's rather optional thing.

I agree that templated name convention makes sense but I'm not sure about $pid - mutex/semaphore ownership is transferred from process to process and with such naming scheme $pid will reflect the pid of creator which might be already dead.
With $pid as a part of mutex template you can use this mutex only to synchronize threads within the same process but not different processes.

source/Host/windows/Mutex.cpp

118–121 ↗

(On Diff #23940)

Removed

137 ↗

(On Diff #23940)

Done

[ovyalov]

• Added mutex name lldb prefix
• Minor fixes in windows::NamedMutex

[zturner]

Windows stuff looks good, only minor comments about the namespace thing.

include/lldb/Host/Mutex.h

260–261 ↗

(On Diff #23940)

Makes sense about the pid. I didn't know about this Global\ Local\ thing before, but I looked it up and seems you're right. If I understand this correctly, then suppose 2 users are connected via Remote Desktop to a Windows machine, and you run LLDB. If you put the mutex in the global namespace, then both users will see the same mutex. If you just use the default though, each will have their own mutex even though the names are the same.

It seems like the latter is what we want. You're trying to solve the race condition in the case of all these different processes running in the same terminal session, so I don't see a need for cross-session synchronization. If I have it correct, then it seems we should remove the Global\.

[zturner]

Ahh, actually maybe we actually do want global in this case. I was thinking that different users would be trying to synchronize among different files, but it would probably be the same file.

[ovyalov]

In D9056#157906, @zturner wrote:

Ahh, actually maybe we actually do want global in this case. I was thinking that different users would be trying to synchronize among different files, but it would probably be the same file.

I think we need to have Global prefix because if an user is logged in simultaneously via RDP and in interactive session he/she uses the same profile directory - so, in case of LLDB temp directory will be %HOMEPATH%\AppData\Temp\.... To make sure that both of them are synchronized on the same mutex to access temp directory Global prefix is required.

[ovyalov]

Let me put on hold this CL since there are some serious issues regarding using POSIX semaphores since they don't recover after process crash - will think more about it and will send a patch later.

[ovyalov]

Decided to stick to file locks since they support process crash recovery both in Posix and Windows implementations.
Main changes:

• Use %UUID%/.lock file for synchronization purposes.
• Download a module to %UUID%/.temp file so iut can be moved to a final file in the same directory without need to copy it.
• Obtain a file lock as a very first step in GetAndPut - just as a safety precaution if rename operation isn't atomic.

[clayborg]

Looks good.

[ovyalov]

AFFECTED FILES

/lldb/trunk/include/lldb/Host/LockFile.h
/lldb/trunk/include/lldb/Host/LockFileBase.h
/lldb/trunk/include/lldb/Host/posix/LockFilePosix.h
/lldb/trunk/include/lldb/Host/windows/LockFileWindows.h
/lldb/trunk/lldb.xcodeproj/project.pbxproj
/lldb/trunk/source/Host/CMakeLists.txt
/lldb/trunk/source/Host/common/LockFileBase.cpp
/lldb/trunk/source/Host/posix/LockFilePosix.cpp
/lldb/trunk/source/Host/windows/LockFileWindows.cpp
/lldb/trunk/source/Target/Platform.cpp
/lldb/trunk/source/Utility/ModuleCache.cpp
/lldb/trunk/source/Utility/ModuleCache.h

USERS

ovyalov (Author)

http://reviews.llvm.org/rL236736

include/lldb/Host/LockFile.h

+//===-- LockFile.h ----------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_LockFile_h_
+#define liblldb_Host_LockFile_h_
+
+#if defined(_WIN32)
+#include "lldb/Host/windows/LockFileWindows.h"
+namespace lldb_private
+{
+typedef LockFileWindows LockFile;
+}
+#else
+#include "lldb/Host/posix/LockFilePosix.h"
+namespace lldb_private
+{
+typedef LockFilePosix LockFile;
+}
+#endif
+
+#endif // liblldb_Host_LockFile_h_

include/lldb/Host/LockFileBase.h

+//===-- LockFileBase.h ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_LockFileBase_h_
+#define liblldb_Host_LockFileBase_h_
+
+#include "lldb/Core/Error.h"
+
+#include <functional>
+
+namespace lldb_private
+{
+
+class LockFileBase
+{
+public:
+    virtual ~LockFileBase () = default;
+
+    bool
+    IsLocked () const;
+
+    Error
+    WriteLock (const uint64_t start, const uint64_t len);
+    Error
+    TryWriteLock (const uint64_t start, const uint64_t len);
+
+    Error
+    ReadLock (const uint64_t start, const uint64_t len);
+    Error
+    TryReadLock (const uint64_t start, const uint64_t len);
+
+    Error
+    Unlock ();
+
+protected:
+    using Locker = std::function<Error (const uint64_t, const uint64_t)>;
+
+    LockFileBase (int fd);
+
+    virtual bool
+    IsValidFile () const;
+
+    virtual Error
+    DoWriteLock (const uint64_t start, const uint64_t len) = 0;
+    virtual Error
+    DoTryWriteLock (const uint64_t start, const uint64_t len) = 0;
+
+    virtual Error
+    DoReadLock (const uint64_t start, const uint64_t len) = 0;
+    virtual Error
+    DoTryReadLock (const uint64_t start, const uint64_t len) = 0;
+
+    virtual Error
+    DoUnlock () = 0;
+
+    Error
+    DoLock (const Locker &locker, const uint64_t start, const uint64_t len);
+
+    int m_fd;  // not owned.
+    bool m_locked;
+    uint64_t m_start;
+    uint64_t m_len;
+};
+
+}
+
+#endif

include/lldb/Host/posix/LockFilePosix.h

+//===-- LockFilePosix.h -----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_posix_LockFilePosix_h_
+#define liblldb_Host_posix_LockFilePosix_h_
+
+#include "lldb/Host/LockFileBase.h"
+
+namespace lldb_private {
+
+class LockFilePosix : public LockFileBase
+{
+public:
+    explicit LockFilePosix (int fd);
+    ~LockFilePosix ();
+
+protected:
+    Error
+    DoWriteLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoTryWriteLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoReadLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoTryReadLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoUnlock () override;
+};
+
+}  // namespace lldb_private
+
+#endif  // liblldb_Host_posix_LockFilePosix_h_

include/lldb/Host/windows/LockFileWindows.h

+//===-- LockFileWindows.h ---------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_posix_LockFileWindows_h_
+#define liblldb_Host_posix_LockFileWindows_h_
+
+#include "lldb/Host/LockFileBase.h"
+#include "lldb/Host/windows/windows.h"
+
+namespace lldb_private {
+
+class LockFileWindows : public LockFileBase
+{
+public:
+    explicit LockFileWindows (int fd);
+    ~LockFileWindows ();
+
+protected:
+    Error
+    DoWriteLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoTryWriteLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoReadLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoTryReadLock (const uint64_t start, const uint64_t len) override;
+
+    Error
+    DoUnlock () override;
+
+    bool
+    IsValidFile () const override;
+
+private:
+    HANDLE m_file;
+};
+
+}  // namespace lldb_private
+
+#endif  // liblldb_Host_posix_LockFileWindows_h_

lldb.xcodeproj/project.pbxproj

/* Begin PBXBuildFile section */
     256CBDBA1ADD107200BC6CDC /* RegisterContextLinux_arm.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 256CBDB61ADD107200BC6CDC /* RegisterContextLinux_arm.cpp */; };
     256CBDBB1ADD107200BC6CDC /* RegisterContextLinux_arm.h in Headers */ = {isa = PBXBuildFile; fileRef = 256CBDB71ADD107200BC6CDC /* RegisterContextLinux_arm.h */; };
     256CBDBC1ADD107200BC6CDC /* RegisterContextLinux_mips64.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 256CBDB81ADD107200BC6CDC /* RegisterContextLinux_mips64.cpp */; };
     256CBDBD1ADD107200BC6CDC /* RegisterContextLinux_mips64.h in Headers */ = {isa = PBXBuildFile; fileRef = 256CBDB91ADD107200BC6CDC /* RegisterContextLinux_mips64.h */; };
     256CBDC01ADD11C000BC6CDC /* RegisterContextPOSIX_arm.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 256CBDBE1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.cpp */; };
     256CBDC11ADD11C000BC6CDC /* RegisterContextPOSIX_arm.h in Headers */ = {isa = PBXBuildFile; fileRef = 256CBDBF1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.h */; };
     257E47171AA56C2000A62F81 /* ModuleCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 257E47151AA56C2000A62F81 /* ModuleCache.cpp */; };
     257E47181AA56C2000A62F81 /* ModuleCache.h in Headers */ = {isa = PBXBuildFile; fileRef = 257E47161AA56C2000A62F81 /* ModuleCache.h */; };
+    25B67C041AE01C910000D19B /* LockFileBase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25B67C031AE01C910000D19B /* LockFileBase.cpp */; };
+    25B67C081AE01CDA0000D19B /* LockFilePosix.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25B67C071AE01CDA0000D19B /* LockFilePosix.cpp */; };
     25EF23781AC09B3700908DF0 /* AdbClient.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25EF23751AC09AD800908DF0 /* AdbClient.cpp */; };
     25EF23791AC09B4200908DF0 /* AdbClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 25EF23761AC09AD800908DF0 /* AdbClient.h */; };
     260157C61885F51C00F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
     260157C81885F53100F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
     2606EDDF184E68A10034641B /* liblldb-core.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 2689FFCA13353D7A00698AC0 /* liblldb-core.a */; };
     260A248E15D06C50009981B0 /* OptionValues.h in Headers */ = {isa = PBXBuildFile; fileRef = 260A248D15D06C4F009981B0 /* OptionValues.h */; };
     260A39A619647A3A004B4130 /* Pipe.h in Headers */ = {isa = PBXBuildFile; fileRef = 260A39A519647A3A004B4130 /* Pipe.h */; };
     260A63171861008E00FECF8E /* IOHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 260A63161861008E00FECF8E /* IOHandler.h */; };
/* Begin PBXFileReference section */
     256CBDB61ADD107200BC6CDC /* RegisterContextLinux_arm.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = RegisterContextLinux_arm.cpp; path = Utility/RegisterContextLinux_arm.cpp; sourceTree = "<group>"; };
     256CBDB71ADD107200BC6CDC /* RegisterContextLinux_arm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterContextLinux_arm.h; path = Utility/RegisterContextLinux_arm.h; sourceTree = "<group>"; };
     256CBDB81ADD107200BC6CDC /* RegisterContextLinux_mips64.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = RegisterContextLinux_mips64.cpp; path = Utility/RegisterContextLinux_mips64.cpp; sourceTree = "<group>"; };
     256CBDB91ADD107200BC6CDC /* RegisterContextLinux_mips64.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterContextLinux_mips64.h; path = Utility/RegisterContextLinux_mips64.h; sourceTree = "<group>"; };
     256CBDBE1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = RegisterContextPOSIX_arm.cpp; path = Utility/RegisterContextPOSIX_arm.cpp; sourceTree = "<group>"; };
     256CBDBF1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterContextPOSIX_arm.h; path = Utility/RegisterContextPOSIX_arm.h; sourceTree = "<group>"; };
     257E47151AA56C2000A62F81 /* ModuleCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ModuleCache.cpp; path = source/Utility/ModuleCache.cpp; sourceTree = "<group>"; };
     257E47161AA56C2000A62F81 /* ModuleCache.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ModuleCache.h; path = source/Utility/ModuleCache.h; sourceTree = "<group>"; };
+    25B67C031AE01C910000D19B /* LockFileBase.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LockFileBase.cpp; sourceTree = "<group>"; };
+    25B67C051AE01CC10000D19B /* LockFile.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = LockFile.h; path = include/lldb/Host/LockFile.h; sourceTree = "<group>"; };
+    25B67C061AE01CC10000D19B /* LockFileBase.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = LockFileBase.h; path = include/lldb/Host/LockFileBase.h; sourceTree = "<group>"; };
+    25B67C071AE01CDA0000D19B /* LockFilePosix.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LockFilePosix.cpp; sourceTree = "<group>"; };
     25EF23751AC09AD800908DF0 /* AdbClient.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AdbClient.cpp; sourceTree = "<group>"; };
     25EF23761AC09AD800908DF0 /* AdbClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AdbClient.h; sourceTree = "<group>"; };
     260157C41885F4FF00F875CF /* libpanel.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libpanel.dylib; path = /usr/lib/libpanel.dylib; sourceTree = "<absolute>"; };
     260223E7115F06D500A601A2 /* SBCommunication.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SBCommunication.h; path = include/lldb/API/SBCommunication.h; sourceTree = "<group>"; };
     260223E8115F06E500A601A2 /* SBCommunication.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBCommunication.cpp; path = source/API/SBCommunication.cpp; sourceTree = "<group>"; };
     26022531115F27FA00A601A2 /* SBFileSpec.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SBFileSpec.h; path = include/lldb/API/SBFileSpec.h; sourceTree = "<group>"; };
     26022532115F281400A601A2 /* SBFileSpec.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBFileSpec.cpp; path = source/API/SBFileSpec.cpp; sourceTree = "<group>"; };
     260A248D15D06C4F009981B0 /* OptionValues.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = OptionValues.h; path = include/lldb/Interpreter/OptionValues.h; sourceTree = "<group>"; };
26BC7DBE10F1B78200F91463 /* Expression */ = {
         49DCF700170E70120092F75E /* Materializer.cpp */,
       );
       name = Expression;
       sourceTree = "<group>";
     };
     26BC7DD010F1B7C100F91463 /* Host */ = {
       isa = PBXGroup;
       children = (
+        25B67C051AE01CC10000D19B /* LockFile.h */,
+        25B67C061AE01CC10000D19B /* LockFileBase.h */,
         6D55B29B1A8CCFF000A70529 /* android */,
         33E5E8451A6736D30024ED68 /* StringConvert.h */,
         69A01E1A1236C5D400C660B5 /* common */,
         3FDFE53919A29399009756A7 /* freebsd */,
         233B009C19610D130090E598 /* linux */,
         26BC7EE510F1B88100F91463 /* MacOSX */,
         3FDFDDC4199D37BE009756A7 /* posix */,
         3FDFE53E19A2940E009756A7 /* windows */,
3F8169261ABB73C1001DA9DF /* Initialization */ = {
         3F8169301ABB7A6D001DA9DF /* SystemLifetimeManager.cpp */,
       );
       name = Initialization;
       sourceTree = "<group>";
     };
     3FDFDDC4199D37BE009756A7 /* posix */ = {
       isa = PBXGroup;
       children = (
+        25B67C071AE01CDA0000D19B /* LockFilePosix.cpp */,
         AFDFDFD019E34D3400EAE509 /* ConnectionFileDescriptorPosix.cpp */,
         3FDFDDC5199D37ED009756A7 /* FileSystem.cpp */,
         3FDFE53019A292F0009756A7 /* HostInfoPosix.cpp */,
         3FDFE53219A29304009756A7 /* HostInfoPosix.h */,
         3FDFE56A19AF9C44009756A7 /* HostProcessPosix.cpp */,
         3FDFE56E19AF9C5A009756A7 /* HostProcessPosix.h */,
         3FDFE56B19AF9C44009756A7 /* HostThreadPosix.cpp */,
         3FDFE56F19AF9C5A009756A7 /* HostThreadPosix.h */,
4CEE62F71145F1C70064CF93 /* GDB Remote */ = {
       );
       name = "GDB Remote";
       path = "gdb-remote";
       sourceTree = "<group>";
     };
     69A01E1A1236C5D400C660B5 /* common */ = {
       isa = PBXGroup;
       children = (
+        25B67C031AE01C910000D19B /* LockFileBase.cpp */,
         250D6AE11A9679270049CC70 /* FileSystem.cpp */,
         33E5E8411A672A240024ED68 /* StringConvert.cpp */,
         25420ED11A649D88009ADBCB /* PipeBase.cpp */,
         26CFDCA2186163A4000E63E5 /* Editline.cpp */,
         260C6EA213011581005E16B0 /* File.cpp */,
         26FA43171301048600E71120 /* FileSpec.cpp */,
         69A01E1B1236C5D400C660B5 /* Condition.cpp */,
         69A01E1C1236C5D400C660B5 /* Host.cpp */,
2689FFC613353D7A00698AC0 /* Sources */ = {
         2689005E13353E0E00698AC0 /* ClangASTSource.cpp in Sources */,
         3FDFED0F19B7D269009756A7 /* ThisThread.cpp in Sources */,
         2689005F13353E0E00698AC0 /* ClangFunction.cpp in Sources */,
         2689006013353E0E00698AC0 /* ClangExpressionDeclMap.cpp in Sources */,
         942AFF0719F84C02007B43B4 /* LibCxxInitializerList.cpp in Sources */,
         2689006113353E0E00698AC0 /* ClangExpressionParser.cpp in Sources */,
         2689006213353E0E00698AC0 /* ClangExpressionVariable.cpp in Sources */,
         2689006313353E0E00698AC0 /* ClangPersistentVariables.cpp in Sources */,
+        25B67C041AE01C910000D19B /* LockFileBase.cpp in Sources */,
         2689006413353E0E00698AC0 /* ClangUserExpression.cpp in Sources */,
         4C3ADCD61810D88B00357218 /* BreakpointResolverFileRegex.cpp in Sources */,
         2689006513353E0E00698AC0 /* ClangUtilityFunction.cpp in Sources */,
         943BDEFE1AA7B2F800789CE8 /* LLDBAssert.cpp in Sources */,
         26474CB418D0CB180073DEBA /* RegisterContextLinux_x86_64.cpp in Sources */,
         2689006613353E0E00698AC0 /* DWARFExpression.cpp in Sources */,
         2689006713353E0E00698AC0 /* ASTDumper.cpp in Sources */,
         2689006813353E0E00698AC0 /* ASTResultSynthesizer.cpp in Sources */,
2689FFC613353D7A00698AC0 /* Sources */ = {
         AF77E0971A033CC70096C0EA /* RegisterContextPOSIXProcessMonitor_powerpc.cpp in Sources */,
         AFDFDFD119E34D3400EAE509 /* ConnectionFileDescriptorPosix.cpp in Sources */,
         9A22A163135E30370024DDC3 /* EmulationStateARM.cpp in Sources */,
         9A4F35101368A51A00823F52 /* StreamAsynchronousIO.cpp in Sources */,
         2692BA15136610C100F9E14D /* UnwindAssemblyInstEmulation.cpp in Sources */,
         263E949F13661AEA00E7D1CE /* UnwindAssembly-x86.cpp in Sources */,
         E7723D481AC4A8C8002BA082 /* RegisterContextFreeBSD_arm64.cpp in Sources */,
         3FDFDDBD199C3A06009756A7 /* FileAction.cpp in Sources */,
+        25B67C081AE01CDA0000D19B /* LockFilePosix.cpp in Sources */,
         264D8D5013661BD7003A368F /* UnwindAssembly.cpp in Sources */,
         AF23B4DB19009C66003E2A58 /* FreeBSDSignals.cpp in Sources */,
         26ECA04313665FED008D1F18 /* ARM_DWARF_Registers.cpp in Sources */,
         AF061F87182C97ED00B6A19C /* RegisterContextHistory.cpp in Sources */,
         AF26703B1852D01E00B6CC36 /* QueueList.cpp in Sources */,
         267C012B136880DF006E963E /* OptionGroupValueObjectDisplay.cpp in Sources */,
         26BCFC521368AE38006DC050 /* OptionGroupFormat.cpp in Sources */,
         23CDD8FB19D47B3600461DDC /* ThreadStateCoordinator.cpp in Sources */,

source/Host/CMakeLists.txt

add_host_subdirectory(common
   common/FileSpec.cpp
   common/FileSystem.cpp
   common/Host.cpp
   common/HostInfoBase.cpp
   common/HostNativeThreadBase.cpp
   common/HostProcess.cpp
   common/HostThread.cpp
   common/IOObject.cpp
+  common/LockFileBase.cpp
   common/Mutex.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeBreakpoint.cpp
   common/NativeBreakpointList.cpp
   common/NativeWatchpointList.cpp
   common/NativeProcessProtocol.cpp
   common/NativeRegisterContext.cpp
   common/NativeRegisterContextRegisterInfo.cpp
add_host_subdirectory(windows
     windows/Condition.cpp
     windows/ConnectionGenericFileWindows.cpp
     windows/EditLineWin.cpp
     windows/FileSystem.cpp
     windows/Host.cpp
     windows/HostInfoWindows.cpp
     windows/HostProcessWindows.cpp
     windows/HostThreadWindows.cpp
+    windows/LockFileWindows.cpp
     windows/Mutex.cpp
     windows/PipeWindows.cpp
     windows/ProcessLauncherWindows.cpp
     windows/ProcessRunLock.cpp
     windows/ThisThread.cpp
     windows/Windows.cpp
     windows/getopt/GetOptInc.cpp
     )
 else()
   add_host_subdirectory(posix
     posix/FileSystem.cpp
     posix/HostInfoPosix.cpp
     posix/HostProcessPosix.cpp
     posix/HostThreadPosix.cpp
+    posix/LockFilePosix.cpp
     posix/PipePosix.cpp
     )
 
   if (NOT __ANDROID_NDK__)
     add_host_subdirectory(posix
       posix/ProcessLauncherPosix.cpp
       )
   endif()

source/Host/common/LockFileBase.cpp

+//===-- LockFileBase.cpp ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/LockFileBase.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace
+{
+
+Error
+AlreadyLocked ()
+{
+    return Error ("Already locked");
+}
+
+Error
+NotLocked ()
+{
+    return Error ("Not locked");
+}
+
+}
+
+LockFileBase::LockFileBase (int fd) :
+    m_fd (fd),
+    m_locked (false),
+    m_start (0),
+    m_len (0)
+{
+
+}
+
+bool
+LockFileBase::IsLocked () const
+{
+    return m_locked;
+}
+
+Error
+LockFileBase::WriteLock (const uint64_t start, const uint64_t len)
+{
+    return DoLock ([&] (const uint64_t start, const uint64_t len)
+                   {
+                       return DoWriteLock (start, len);
+                   }, start, len);
+}
+
+Error
+LockFileBase::TryWriteLock (const uint64_t start, const uint64_t len)
+{
+    return DoLock ([&] (const uint64_t start, const uint64_t len)
+                   {
+                        return DoTryWriteLock (start, len);
+                   }, start, len);
+}
+
+Error
+LockFileBase::ReadLock (const uint64_t start, const uint64_t len)
+{
+    return DoLock ([&] (const uint64_t start, const uint64_t len)
+                   {
+                        return DoReadLock (start, len);
+                   }, start, len);
+}
+
+Error
+LockFileBase::TryReadLock (const uint64_t start, const uint64_t len)
+{
+    return DoLock ([&] (const uint64_t start, const uint64_t len)
+                   {
+                       return DoTryReadLock (start, len);
+                   }, start, len);
+
+}
+
+Error
+LockFileBase::Unlock ()
+{
+    if (!IsLocked ())
+        return NotLocked ();
+
+    const auto error = DoUnlock ();
+    if (error.Success ())
+    {
+        m_locked = false;
+        m_start = 0;
+        m_len = 0;
+    }
+    return error;
+}
+
+bool
+LockFileBase::IsValidFile () const
+{
+    return m_fd != -1;
+}
+
+Error
+LockFileBase::DoLock (const Locker &locker, const uint64_t start, const uint64_t len)
+{
+    if (!IsValidFile ())
+        return Error("File is invalid");
+
+    if (IsLocked ())
+        return AlreadyLocked ();
+
+    const auto error = locker (start, len);
+    if (error.Success ())
+    {
+        m_locked = true;
+        m_start = start;
+        m_len = len;
+    }
+
+    return error;
+}

source/Host/posix/LockFilePosix.cpp

+//===-- LockFilePosix.cpp ---------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/posix/LockFilePosix.h"
+
+#include <fcntl.h>
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace
+{
+
+Error fileLock (int fd, int cmd, int lock_type, const uint64_t start, const uint64_t len)
+{
+    struct flock fl;
+
+    fl.l_type   = lock_type;
+    fl.l_whence = SEEK_SET;
+    fl.l_start  = start;
+    fl.l_len    = len;
+    fl.l_pid    = ::getpid ();
+
+    Error error;
+    if (::fcntl (fd, cmd, &fl) == -1)
+        error.SetErrorToErrno ();
+
+    return error;
+}
+
+}  // namespace
+
+LockFilePosix::LockFilePosix (int fd)
+    : LockFileBase (fd)
+{
+}
+
+LockFilePosix::~LockFilePosix ()
+{
+    Unlock ();
+}
+
+Error
+LockFilePosix::DoWriteLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_fd, F_SETLKW, F_WRLCK, start, len);
+}
+
+Error
+LockFilePosix::DoTryWriteLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_fd, F_SETLK, F_WRLCK, start, len);
+}
+
+Error
+LockFilePosix::DoReadLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_fd, F_SETLKW, F_RDLCK, start, len);
+}
+
+Error
+LockFilePosix::DoTryReadLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_fd, F_SETLK, F_RDLCK, start, len);
+}
+
+Error
+LockFilePosix::DoUnlock ()
+{
+    return fileLock (m_fd, F_SETLK, F_UNLCK, m_start, m_len);
+}

source/Host/windows/LockFileWindows.cpp

+//===-- LockFileWindows.cpp -------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/windows/LockFileWindows.h"
+
+#include "lldb/Host/windows/AutoHandle.h"
+
+#include <io.h>
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace
+{
+
+Error fileLock (HANDLE file_handle, DWORD flags, const uint64_t start, const uint64_t len)
+{
+    if (start != 0)
+        return Error ("Non-zero start lock regions are not supported");
+
+    OVERLAPPED overlapped = {0};
+    const AutoHandle event_handle (::CreateEvent (nullptr, TRUE, FALSE, nullptr));
+    overlapped.hEvent = event_handle.get ();
+
+    if (!::LockFileEx (file_handle, flags, 0, len, 0, &overlapped) && ::GetLastError () != ERROR_IO_PENDING)
+        return Error (::GetLastError (), eErrorTypeWin32);
+
+    if (::WaitForSingleObject (overlapped.hEvent, INFINITE) != WAIT_OBJECT_0)
+        return Error (::GetLastError (), eErrorTypeWin32);
+
+    return Error ();
+}
+
+}  // namespace
+
+LockFileWindows::LockFileWindows (int fd)
+    : LockFileBase (fd),
+      m_file (reinterpret_cast<HANDLE> (_get_osfhandle (fd)))
+{
+}
+
+LockFileWindows::~LockFileWindows ()
+{
+    Unlock ();
+    if (m_file != INVALID_HANDLE_VALUE)
+    {
+        _close (reinterpret_cast<int> (m_file));
+        m_file = INVALID_HANDLE_VALUE;
+    }
+}
+
+bool
+LockFileWindows::IsValidFile () const
+{
+    return LockFileBase::IsValidFile() && m_file != INVALID_HANDLE_VALUE;
+}
+
+Error
+LockFileWindows::DoWriteLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_file, LOCKFILE_EXCLUSIVE_LOCK, start, len);
+}
+
+Error
+LockFileWindows::DoTryWriteLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_file, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, start, len);
+}
+
+Error
+LockFileWindows::DoReadLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_file, 0, start, len);
+}
+
+Error
+LockFileWindows::DoTryReadLock (const uint64_t start, const uint64_t len)
+{
+    return fileLock (m_file, LOCKFILE_FAIL_IMMEDIATELY, start, len);
+}
+
+Error
+LockFileWindows::DoUnlock ()
+{
+    OVERLAPPED overlapped = {0};
+    const AutoHandle event_handle (::CreateEvent (nullptr, TRUE, FALSE, nullptr));
+    overlapped.hEvent = event_handle.get ();
+
+    if (!::UnlockFileEx (file_handle, 0, m_len, 0, &overlapped) && ::GetLastError () != ERROR_IO_PENDING)
+        return Error (::GetLastError (), eErrorTypeWin32);
+
+    if (::WaitForSingleObject (overlapped.hEvent, INFINITE) != WAIT_OBJECT_0)
+        return Error (::GetLastError (), eErrorTypeWin32);
+
+    return Error ();
+}

source/Utility/ModuleCache.h

GetAndPut(const FileSpec &root_dir_spec,
               const Downloader &downloader,
               lldb::ModuleSP &cached_module_sp,
               bool *did_create_ptr);
 
 private:
     static FileSpec
     GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid);
 
+    static Error
+    GetModuleLockFilePath (const FileSpec &root_dir_spec, const UUID &uuid, FileSpec &lock_file_path);
+
     static FileSpec
     GetHostSysRootModulePath (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec);
 
     static Error
     CreateHostSysRootModuleSymLink (const FileSpec &sysroot_module_path_spec, const FileSpec &module_file_path);
 
     std::unordered_map<std::string, lldb::ModuleWP> m_loaded_modules;
 };
 
 } // namespace lldb_private
 
 #endif  // utility_ModuleCache_h_

source/Utility/ModuleCache.cpp

 //===--------------------- ModuleCache.cpp ----------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ModuleCache.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/LockFile.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 
 #include <assert.h>
 
 #include <cstdio>
 #
 using namespace lldb;
 using namespace lldb_private;
 
 namespace {
 
 const char* kModulesSubdir = ".cache";
+const char* kLockSubDir = ".lock";
 
 FileSpec
 JoinPath (const FileSpec &path1, const char* path2)
 {
     FileSpec result_spec (path1);
     result_spec.AppendPathComponent (path2);
     return result_spec;
 }
ModuleCache::GetAndPut (const FileSpec &root_dir_spec,
     auto error = Get (root_dir_spec,
                       hostname,
                       module_spec,
                       cached_module_sp,
                       did_create_ptr);
     if (error.Success ())
         return error;
 
+    FileSpec lock_file_path;
+    error = GetModuleLockFilePath (root_dir_spec, module_spec.GetUUID (), lock_file_path);
+    if (error.Fail ())
+        return Error("Failed to get module lock path: %s", error.AsCString ());
+
+    File lock_file (lock_file_path, File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+    if (!lock_file.IsValid ())
+    {
+        error.SetErrorToErrno ();
+        return Error("Failed to open lock file %s: %s", lock_file_path.GetPath ().c_str (), error.AsCString ());
+    }
+    llvm::FileRemover lock_file_remover (lock_file_path.GetPath ().c_str ());
+    LockFile lock (lock_file.GetDescriptor ());
+    error = lock.WriteLock (0, 1);
+    if (error.Fail ())
+        return Error("Failed to lock file %s:%s", lock_file_path.GetPath ().c_str (), error.AsCString ());
+
+    // Check local cache for a module again.
+    error = Get (root_dir_spec, hostname, module_spec, cached_module_sp, did_create_ptr);
+    if (error.Success ())
+        return error;
+
     FileSpec tmp_download_file_spec;
     error = downloader (module_spec, tmp_download_file_spec);
     llvm::FileRemover tmp_file_remover (tmp_download_file_spec.GetPath ().c_str ());
     if (error.Fail ())
         return Error("Failed to download module: %s", error.AsCString ());
 
     // Put downloaded file into local module cache.
     error = Put (root_dir_spec,

 
 FileSpec
 ModuleCache::GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid)
 {
     const auto modules_dir_spec = JoinPath (root_dir_spec, kModulesSubdir);
     return JoinPath (modules_dir_spec, uuid.GetAsString ().c_str ());
 }
 
+Error
+ModuleCache::GetModuleLockFilePath (const FileSpec &root_dir_spec, const UUID &uuid, FileSpec &lock_file_path)
+{
+    const auto lock_dir_spec = JoinPath (root_dir_spec, kLockSubDir);
+    auto error = MakeDirectory (lock_dir_spec);
+    if (error.Fail ())
+        return error;
+
+    lock_file_path = JoinPath (lock_dir_spec, uuid.GetAsString ().c_str ());
+    return error;
+}
+
 FileSpec
 ModuleCache::GetHostSysRootModulePath (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec)
 {
     const auto sysroot_dir = JoinPath (root_dir_spec, hostname);
     return JoinPath (sysroot_dir, platform_module_spec.GetPath ().c_str ());
 }
 
 Error