Patchwork [STABLE] test-push-http: do not clear pid file

login
register
mail settings
Submitter Yuya Nishihara
Date April 26, 2018, 12:34 p.m.
Message ID <7dffecb148aa77184b19.1524746050@mimosa>
Download mbox | patch
Permalink /patch/31236/
State Accepted
Headers show

Comments

Yuya Nishihara - April 26, 2018, 12:34 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1524744656 -32400
#      Thu Apr 26 21:10:56 2018 +0900
# Branch stable
# Node ID 7dffecb148aa77184b19c054e301345b3b51af1b
# Parent  da07c781aba9ef88cbf76d07c6a6e2ec40cf6693
test-push-http: do not clear pid file

It's okay now, but we'll end up leaking daemon processes if we add some
more.
Augie Fackler - April 26, 2018, 5:17 p.m.
> On Apr 26, 2018, at 08:34, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1524744656 -32400
> #      Thu Apr 26 21:10:56 2018 +0900
> # Branch stable
> # Node ID 7dffecb148aa77184b19c054e301345b3b51af1b
> # Parent  da07c781aba9ef88cbf76d07c6a6e2ec40cf6693
> test-push-http: do not clear pid file

queued, thanks
Gregory Szorc - April 26, 2018, 5:27 p.m.
> On Apr 26, 2018, at 10:17, Augie Fackler <raf@durin42.com> wrote:
> 
> 
> 
>> On Apr 26, 2018, at 08:34, Yuya Nishihara <yuya@tcha.org> wrote:
>> 
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1524744656 -32400
>> #      Thu Apr 26 21:10:56 2018 +0900
>> # Branch stable
>> # Node ID 7dffecb148aa77184b19c054e301345b3b51af1b
>> # Parent  da07c781aba9ef88cbf76d07c6a6e2ec40cf6693
>> test-push-http: do not clear pid file
> 
> queued, thanks

Oops. Sorry about the oversight. I usually call killdaemons.py liberally and then overwrite the pids file since leaving a reference to a killed pid would be problematic if there were a future pid collision.

I do wonder if we should add functionality to the test harness so ‘hg serve -d’ automagically updates a pid file and killdaemons.py nukes the pid file so we don’t have to do all this management. Strangely, killdaemons.py has the remove file functionality but it isn’t used AFAICT.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - April 27, 2018, 11:11 a.m.
On Thu, 26 Apr 2018 10:27:32 -0700, Gregory Szorc wrote:
> > On Apr 26, 2018, at 10:17, Augie Fackler <raf@durin42.com> wrote:
> >> On Apr 26, 2018, at 08:34, Yuya Nishihara <yuya@tcha.org> wrote:
> >> 
> >> # HG changeset patch
> >> # User Yuya Nishihara <yuya@tcha.org>
> >> # Date 1524744656 -32400
> >> #      Thu Apr 26 21:10:56 2018 +0900
> >> # Branch stable
> >> # Node ID 7dffecb148aa77184b19c054e301345b3b51af1b
> >> # Parent  da07c781aba9ef88cbf76d07c6a6e2ec40cf6693
> >> test-push-http: do not clear pid file
> > 
> > queued, thanks
> 
> Oops. Sorry about the oversight. I usually call killdaemons.py liberally and then overwrite the pids file since leaving a reference to a killed pid would be problematic if there were a future pid collision.

Maybe killdaemons.py should do killdaemons(remove=True) by default?

FWIW, it might cause a problem on Windows to kill and start new daemon
frequently because SO_REUSEADDR is off.

> I do wonder if we should add functionality to the test harness so ‘hg serve -d’ automagically updates a pid file and killdaemons.py nukes the pid file so we don’t have to do all this management. Strangely, killdaemons.py has the remove file functionality but it isn’t used AFAICT.

Sounds nice.
Joerg Sonnenberger - April 27, 2018, 1:24 p.m.
On Fri, Apr 27, 2018 at 08:11:58PM +0900, Yuya Nishihara wrote:
> FWIW, it might cause a problem on Windows to kill and start new daemon
> frequently because SO_REUSEADDR is off.

Does Windows leave behind purely local sockets? I know that Linux has
that bug.

Joerg
Yuya Nishihara - April 28, 2018, 1:17 a.m.
On Fri, 27 Apr 2018 15:24:27 +0200, Joerg Sonnenberger wrote:
> On Fri, Apr 27, 2018 at 08:11:58PM +0900, Yuya Nishihara wrote:
> > FWIW, it might cause a problem on Windows to kill and start new daemon
> > frequently because SO_REUSEADDR is off.
> 
> Does Windows leave behind purely local sockets? I know that Linux has
> that bug.

I don't know the detail. I just meant the port might yet be reusable as the
server is killed by TerminateProcess().
Matt Harbison - April 28, 2018, 1:49 a.m.
On Thu, 26 Apr 2018 13:27:32 -0400, Gregory Szorc  
<gregory.szorc@gmail.com> wrote:

>
>
>> On Apr 26, 2018, at 10:17, Augie Fackler <raf@durin42.com> wrote:
>>
>>
>>
>>> On Apr 26, 2018, at 08:34, Yuya Nishihara <yuya@tcha.org> wrote:
>>>
>>> # HG changeset patch
>>> # User Yuya Nishihara <yuya@tcha.org>
>>> # Date 1524744656 -32400
>>> #      Thu Apr 26 21:10:56 2018 +0900
>>> # Branch stable
>>> # Node ID 7dffecb148aa77184b19c054e301345b3b51af1b
>>> # Parent  da07c781aba9ef88cbf76d07c6a6e2ec40cf6693
>>> test-push-http: do not clear pid file
>>
>> queued, thanks
>
> Oops. Sorry about the oversight. I usually call killdaemons.py liberally  
> and then overwrite the pids file since leaving a reference to a killed  
> pid would be problematic if there were a future pid collision.
>
> I do wonder if we should add functionality to the test harness so ‘hg  
> serve -d’ automagically updates a pid file and killdaemons.py nukes the  
> pid file so we don’t have to do all this management. Strangely,  
> killdaemons.py has the remove file functionality but it isn’t used  
> AFAICT.

I've wondered that too.  Somewhere recently, I read that Windows tends to  
reuse its PIDs sooner than Linux would.  I've wondered with the PIDs  
hanging around, if we ever kill the wrong thing in tests that use  
killdaemons multiple times.  I saw you do this, and thought it was a  
better way, so I probably did this myself recently too.

I also wonder, given the persistent nagging address in use failure on  
Windows when relaunching `hg serve` in some tests, if we should just have  
some way of sending an exit command over the socket, so that it can  
shutdown gracefully.  Obviously, it would have to be a test extension.
Yuya Nishihara - April 28, 2018, 2:39 a.m.
On Fri, 27 Apr 2018 21:49:33 -0400, Matt Harbison wrote:
> I also wonder, given the persistent nagging address in use failure on  
> Windows when relaunching `hg serve` in some tests, if we should just have  
> some way of sending an exit command over the socket, so that it can  
> shutdown gracefully.  Obviously, it would have to be a test extension.

I can upstream the win32ill extension, which makes hg listen to WM_CLOSE,
but I have no idea if it works well with the win32.setsignalhandler() hack.

https://bitbucket.org/tortoisehg/thg/src/tip/tortoisehg/util/win32ill.py
Matt Harbison - April 28, 2018, 4:42 a.m.
On Fri, 27 Apr 2018 22:39:32 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Fri, 27 Apr 2018 21:49:33 -0400, Matt Harbison wrote:
>> I also wonder, given the persistent nagging address in use failure on
>> Windows when relaunching `hg serve` in some tests, if we should just  
>> have
>> some way of sending an exit command over the socket, so that it can
>> shutdown gracefully.  Obviously, it would have to be a test extension.
>
> I can upstream the win32ill extension, which makes hg listen to WM_CLOSE,
> but I have no idea if it works well with the win32.setsignalhandler()  
> hack.
>
> https://bitbucket.org/tortoisehg/thg/src/tip/tortoisehg/util/win32ill.py

I'll try playing with it this weekend.  Maybe it can be used somehow to  
get showstack.py to work on Windows too?

I see the caveat about processes that communicate via stdio... does that  
mean the ssh stuff will have an issue?  (I have no idea how that works,  
but when I was debugging the infinite push stuff, it really screwed things  
up using print() instead of ui.status().)
Yuya Nishihara - April 28, 2018, 9:47 a.m.
On Sat, 28 Apr 2018 00:42:16 -0400, Matt Harbison wrote:
> On Fri, 27 Apr 2018 22:39:32 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Fri, 27 Apr 2018 21:49:33 -0400, Matt Harbison wrote:
> >> I also wonder, given the persistent nagging address in use failure on
> >> Windows when relaunching `hg serve` in some tests, if we should just  
> >> have
> >> some way of sending an exit command over the socket, so that it can
> >> shutdown gracefully.  Obviously, it would have to be a test extension.
> >
> > I can upstream the win32ill extension, which makes hg listen to WM_CLOSE,
> > but I have no idea if it works well with the win32.setsignalhandler()  
> > hack.
> >
> > https://bitbucket.org/tortoisehg/thg/src/tip/tortoisehg/util/win32ill.py
> 
> I'll try playing with it this weekend.  Maybe it can be used somehow to  
> get showstack.py to work on Windows too?

No idea if we have another option to interrupt the main thread.

> I see the caveat about processes that communicate via stdio... does that  
> mean the ssh stuff will have an issue?

That comment is about blocking read(), so I think it's unrelated to the
issue you faced.

> (I have no idea how that works,
> but when I was debugging the infinite push stuff, it really screwed things
> up using print() instead of ui.status().)

I have unsent patch to fully redirect stdout to stderr, which will fix the
issue.

Patch

diff --git a/tests/test-push-http.t b/tests/test-push-http.t
--- a/tests/test-push-http.t
+++ b/tests/test-push-http.t
@@ -398,7 +398,7 @@  Pushing via hgwebdir works
   > EOF
 
   $ hg serve --web-conf web.conf -p $HGPORT -d --pid-file hg.pid
-  $ cat hg.pid > $DAEMON_PIDS
+  $ cat hg.pid >> $DAEMON_PIDS
 
   $ hg clone http://localhost:$HGPORT/hgwebdir hgwebdir-local
   requesting all changes