This is an archive of the discontinued LLVM Phabricator instance.

[lldb-vscode] Fix data race in lldb-vscode when running with ThreadSanitizer
ClosedPublic

Authored by PRESIDENT810 on May 5 2022, 8:50 PM.

Details

Summary

This patch fixes https://github.com/llvm/llvm-project/issues/54768. A ProgressEventReporter creates a dedicated thread that keeps checking whether there are new events that need to be sent to IDE as long as m_thread_should_exit is true. When the VSCode instance is destructed, it will set m_thread_should_exit to false, which caused a data race because at the same time its ProgressEventReporter is reading this value to determine whether it should quit. This fix simply uses mutex to ensure they cannot read and write this value at the same time.

Diff Detail

Event Timeline

PRESIDENT810 created this revision.May 5 2022, 8:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2022, 8:50 PM
PRESIDENT810 requested review of this revision.May 5 2022, 8:50 PM
wallace accepted this revision.May 9 2022, 3:43 PM

Thanks for doing this fix. I hope tsan doesn't report this anymore

This revision is now accepted and ready to land.May 9 2022, 3:43 PM

Another solution would be to make m_thread_should_exit a std::atomic<bool> type instead of just a bool.

Update to using std::atomic to avoid data race instead of mutex lock.

clayborg accepted this revision.May 11 2022, 3:19 PM

Much easier and cleaner. LGTM

Hi, I just forgot to mention that I don't really have access to commit... So if any people see this and this patch looks good to them, maybe please help me commit this one?

Hi! I'll push it for you :)

Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 9:11 AM