| 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
> 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
> 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
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.
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
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().
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.
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
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().)
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