[libc-commits] [PATCH] D76676: [libc] Unblock SIGABRT before raising in abort

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/stdlib/abort.cpp:34
+  // interrupted by any async signals. The user handler will be run once we
+  // unblock if it was blocked previously.
+  sigset_t set;
----------------
MaskRay wrote:
> sivachandra wrote:
> > MaskRay wrote:
> > > abrachet wrote:
> > > > MaskRay wrote:
> > > > > Another thread may concurrently install a signal handler for SIGABRT or unblock SIGABRT. There needs to be a lock.
> > > > Yes this was discussed, we need a recursive lock here but also in sigaction, we don't have those yet. I don't personally think the benefits are worth that complexity anyway though.
> > > This suggests that the patches should be applied in a better way. If a mutex is implemented before improving abort, then we don't need to care about this.
> > > 
> > > I think a non-recursive mutex suffices here.
> > Perfect is the enemy of the good: https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good
> > FWIW, this albeit incomplete implementation allowed us to make progress with other pieces. Not to claim other pieces are complete or perfect. As I said in another post, the quest for perfection hinders progress, and more importantly, hinders our own learning.
> I am not sure this is the excuse to check in the imperfect code at this point. Actually the simply abort() is sufficient for 99.9% use cases. Nobody will sigaction SIGABRT to abort itself. When the other infrastructure is not ready, it is premature to over-optimize abort. You can disregard it but that will eventually clutter up the history of llvm-libc.
> 
> A messy history will just hinder learning for posterity, which may be a lot of people.
You seem to argue for perfection and imperfection at the same time (because this patch is trying to reduce the existing imperfection). It is probably getting into subjective opinions at this point so I am going to let @alexbrachet decide if they want to submit it as is, or if they want to wait for a recursive mutex implementation. I am OK with either approach as I don't think "clutter up history" is an issue.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76676/new/

https://reviews.llvm.org/D76676