From owner-freebsd-current@freebsd.org Wed Jul 13 19:19:53 2016 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 9ED5AB9837B for ; Wed, 13 Jul 2016 19:19:53 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 376F61D17; Wed, 13 Jul 2016 19:19:53 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id u6DJJlCn080299 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 13 Jul 2016 22:19:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua u6DJJlCn080299 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id u6DJJl1p080298; Wed, 13 Jul 2016 22:19:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 13 Jul 2016 22:19:47 +0300 From: Konstantin Belousov To: Mark Johnston Cc: freebsd-current@FreeBSD.org Subject: Re: ptrace attach in multi-threaded processes Message-ID: <20160713191947.GW38613@kib.kiev.ua> References: <20160712011938.GA51319@wkstn-mjohnston.west.isilon.com> <20160712055753.GI38613@kib.kiev.ua> <20160712170502.GA71220@wkstn-mjohnston.west.isilon.com> <20160712175150.GP38613@kib.kiev.ua> <20160712182414.GC71220@wkstn-mjohnston.west.isilon.com> <20160713033036.GR38613@kib.kiev.ua> <20160713040210.GA89573@wkstn-mjohnston.west.isilon.com> <20160713045439.GT38613@kib.kiev.ua> <20160713164247.GA2066@wkstn-mjohnston.west.isilon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160713164247.GA2066@wkstn-mjohnston.west.isilon.com> User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jul 2016 19:19:53 -0000 On Wed, Jul 13, 2016 at 09:42:47AM -0700, Mark Johnston wrote: > On Wed, Jul 13, 2016 at 07:54:39AM +0300, Konstantin Belousov wrote: > > I finally see. Might be something like the patch below is a step in > > the desired direction. Idea is in the proc_next_xthread(): p_xthread > > should be set to the next thread with a pending signal. Do you have a > > test case that demonstrates the issue ? > > Not yet, I'll work on one. I've only seen this occur once in an Isilon > test cluster. > > The diff makes sense to me, thanks. I'd find the code easier to read if > proc_next_xthread() was a pure function that returned the flagged > thread instead of setting p_xthread. I did coded it this way, struct thread *proc_next_xthread(struct proc *p); initially, but I did not liked that all uses are p->p_xthread = proc_next_xthread(p); > > I'm having trouble determining if the diff changes any userland-visible > behaviour. It seems that the only potential problem with the current > p_xthread handling is in stopevent(), since a thread calling stopevent() > from postsig() may clear p_xthread after it was set by another thread in > ptracestop(). But I also don't understand why we call stopevent(S_SIG) > from both issignal() and postsig() - this would appear to stop the > thread twice for the same signal. You mean that the patch would not fix your issue ? Quite possible, it might require some more code to 'move the torch' to next xthread, so to say. When you write the test case, I will spend efforts on the working patch. That said, I do not think that we should change anything about stopevent(), since this is code which is on life support. If we cannot remove procfs debugging interface, let not change it at least in incompatible ways. > > With respect to the desired direction, do you agree that the SIGSTOP > from PT_ATTACH should effectively be ignored if a different signal stops > the process first? As I said in a previous post, it seems that the > SA_STOP property of PT_ATTACH's SIGSTOP is not used in the common case, > since ptracestop() will stop the process if any signal is received, and > the PT_DETACH operation will typically overwrite the SIGSTOP with 0 in > td_xsig. Hmm, I think no, we can not make such change. Issue is, debugger interface guarantees (at least for single-threaded programs it is done correctly) that SIGSTOP is noted. In my opinion, it would be the incompatible API change.