bulbazord added a comment. In D159011#4624495 <https://reviews.llvm.org/D159011#4624495>, @fdeazeve wrote:
> I am slightly wary of making this a global set without any kind of thread > safe mechanism, as I (naively, not really knowing how this class is used) > would expect us to be able to instantiate multiple servers and have them be > accessed concurrently. As such, it makes more sense to have each server own > its set of strings. > > Have we considered having UnixSignals be the one to extend the lifetime of > strings? It seems that UnixSignals is the one storing the StringRefs and > therefore requiring extended lifetime. I couldn't figure out if there is any > other advantage to this vs the other suggestion. Right, that's a fair concern I think. The reason I chose to make it not the job of `UnixSignals` is that the majority of these strings are string literals. I think having the one instance where the strings are not literals somewhere in a data section should be responsible for guaranteeing the lifetimes of these strings. I did not want to put them in the ConstString string pool because I don't think that string pool needs any more strings in it than already exist there. That being said, you suggested the StringSet be a private member variable of `PlatformRemoteGDBServer`. This may work, but I wasn't sure what the lifecycle of `PlatformRemoteGDBServer` objects were. Perhaps it exists for a time but is destroyed sometime later. At that point, UnixSignals will hold onto some dangling references. I decided to make it a global object to guarantee its lifetime independent of any `PlatformRemoteGDBServer` instance. I don't mind wrapping it in a concurrent structure to guarantee thread safety though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159011/new/ https://reviews.llvm.org/D159011 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
