Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-09-26 Thread Eugene Zelenko via lldb-commits
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Looks like patch was not committed. https://reviews.llvm.org/D17635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-19 Thread Zachary Turner via lldb-commits
zturner added a comment. I can remove that comment for you, no worries. http://reviews.llvm.org/D17635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-19 Thread Zachary Turner via lldb-commits
zturner added a comment. Petr, is this one ok to go in? http://reviews.llvm.org/D17635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-19 Thread Petr Hons via lldb-commits
Honsik added a comment. Yes please, with that comment change jingham has mentioned. Do you want me to create new diff? http://reviews.llvm.org/D17635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-10 Thread Jim Ingham via lldb-commits
jingham added a comment. More precisely, the "Public running lock..." part of the comment. http://reviews.llvm.org/D17635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-10 Thread Jim Ingham via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I marked a comment left over from a previous draft of the patch that isn't needed. Other than that, this looks fine. Comment at: source/Target/Process.cpp:3561-3562 @@

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-03-10 Thread Petr Hons via lldb-commits
Honsik updated this revision to Diff 50373. Honsik added a comment. Sorry for the delay, I have been on holiday. I have modified the patch as jingham requested. The public run lock is reset back on error in both Resume and ResumeSynchronous methods. http://reviews.llvm.org/D17635 Files:

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-02-26 Thread Jim Ingham via lldb-commits
jingham added a comment. I don't think you can manipulate the public run lock in PrivateResume like this. PrivateResume gets run in a bunch of places (like calling functions) that are way below the level the public run lock. You probably need to catch errors from PrivateResume in Resume and

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-02-26 Thread Jim Ingham via lldb-commits
jingham requested changes to this revision. jingham added a reviewer: jingham. jingham added a comment. This revision now requires changes to proceed. I agree with Zachary, it would be better to put it in PrivateResume before the call to WillResume. Having this happen in Process::PrivateResume

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-02-26 Thread Zachary Turner via lldb-commits
zturner added a comment. It doesn't look like `Process::PrivateResume()` returns an error if the process is alive unless `WillResume()` returns an error, which is up to the individual process implementation. So maybe the short circuit needs to happen there. This isn't really my area though

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-02-26 Thread Petr Hons via lldb-commits
Honsik added a comment. I tried to put this check in PrivateResume, but its not that simple because of the public RUN lock. I am not that sure if it is safe to always unclock the lock inside PrivateResume. http://reviews.llvm.org/D17635 ___

Re: [Lldb-commits] [PATCH] D17635: Continue after process exit

2016-02-26 Thread Jim Ingham via lldb-commits
jingham added a subscriber: jingham. jingham added a comment. It's okay to short-circuit this here, but why was PrivateResume not returning an error when the process was not alive. That error should have gotten propagated to the caller, obviating the need for this short-circuit.