Subject: [sup-talk] sup ignoring SIGTERM?

From: [email-masked] (Adeodato Simó)
Date: Wed, 05 Aug 2009 16:09:26 +0200

Hello,

it is my impression that sup is ignoring SIGTERM, whereas I would be
expecting for sup to clean up (including removal of the lock file) and
exit normally upon receiving this signal.

Any clues about this? (I tried inserting a "trap", but it didn't seem to
help.)

Thanks,

-- 
- Are you sure we're good?
- Always.
        -- Rory and Lorelai

---

From: [email-masked] (William Morgan)
Date: Wed, 05 Aug 2009 13:03:09 -0700

> exit normally upon receiving this signal.

Good point. I've fixed that in branch ncurses-fixes, which I've merged
into next. Let me know if it works for you.

I'm also happy to report that I've finally managed to get Sup to deal
with screen resizes, which, oddly enough, was a related problem. Go me!
Steps closer to curses-induced insanity: 4!

Available on next or ncurses-fixes branches.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Nicolas Pouillard)
Date: Wed, 05 Aug 2009 22:25:49 +0200

> Available on next or ncurses-fixes branches.

With the ncurses-fixes branch (even with the last patch), when I launch sup
I get a blank screen (with the status bar saying "Searching for threads...".

-- 
Nicolas Pouillard
http://nicolaspouillard.fr

---

From: [email-masked] (Ben Walton)
Date: Wed, 05 Aug 2009 16:35:51 -0400

> into next. Let me know if it works for you.

I just pulled next and fired it up.  It was _hoooooribly_,
_bruuuutallly_ slow to load the index.  I git bisected between my
previous running ORIG_HEAD and HEAD.  I came up with 75d9f5e3db as the
offender.  It seems to do what you intend but (for me, anyway) at the
expense of really bad interactivity.  [We're talking 10+ minutes to
load the index part way...]

This doesn't help identify the problem, but I'd be looking at the
Ncurses.nodelay bits...

Thanks
-Ben
-- 
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302

GPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu
Contact me to arrange for a CAcert assurance meeting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20090805/b4062212/attachment.bin>

---

From: [email-masked] (William Morgan)
Date: Wed, 05 Aug 2009 14:24:49 -0700

Reformatted excerpts from Nicolas Pouillard's message of 2009-08-05:
> With the ncurses-fixes branch (even with the last patch), when I
> launch sup I get a blank screen (with the status bar saying "Searching
> for threads...".

Dammit. I see that too, on some computers. The problem is that
Ncurses.getch is actually blocking the background threads from working.
(You can tell because threads will start loading when you press lots of
keys.)

I've unmerged the branch from next for the time being.

The problem is the removal of the select() in nonblocking_getch
(buffer.rb circa line 25). With it in, resizes don't work right. With it
removed, background threads are blocked on some computers. Not sure what
makes the difference. Sigh...
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Edward Z. Yang)
Date: Wed, 05 Aug 2009 17:28:43 -0400

> Steps closer to curses-induced insanity: 4!

Sounds exciting. What was the fix?

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Thu, 06 Aug 2009 10:53:43 -0700

Reformatted excerpts from William Morgan's message of 2009-08-05:
> (buffer.rb circa line 25). With it in, resizes don't work right. With
> it removed, background threads are blocked on some computers. Not sure
> what makes the difference. Sigh...

Ok, I've tracked this down. FWIW, this is a problem that's been around
in ruby ncurses libraries since at least 2005, when I complained about
it on ruby-core.

In many ruby ncurses libraries, including the most recent gem,
Ncurses.getch blocks all Ruby threads from running. That's a bug. The
workaround is to wrap it in a select() call. But then you have to handle
sigwinch manually (i.e via Kernel#trap), because getch won't return a
resize "keystroke" until the user hits an actual key.

The libncurses-ruby library on modern Ubuntu systems actually doesn't
have this problem. You can set nodelay false, throw out the select call,
and everything works the way it's supposed to: background threads run,
and you get a resize keystroke when a sigwinch happens without you
having to install your own handler. And that's the library I had been
developing against, which is why everything worked on one computer but
didn't work on the others.

Anyways, for those who were having problems with this branch before,
please try the most recent version and confirm that:
1. Email threads load as normal, i.e. in the background.
2. Resizing the terminal refreshes the screen without you having to
  press any keys.
3. Ctrl-l still works.
4. Ctrl-c still works (prompts you, and y/n do the right thing).
5. Killing Sup works.

If everyone's happy, I'll merge this branch into next.

If you're curious about whether you have a good or bad ncurses library,
run this program: (But either should work correctly with Sup.)

  require 'rubygems'
  require 'ncurses'
  require 'thread'

  Ncurses.initscr
  Ncurses.noecho
  Ncurses.cbreak
  Ncurses.curs_set 0

  Thread.new do
    sleep 0.1
    Ncurses.stdscr.mvaddstr 0, 0, "ncurses library is GOOD."
  end

  begin
    Ncurses.stdscr.mvaddstr 0, 0, "ncurses library is BAD."
    Ncurses.getch
  ensure
    Ncurses.curs_set 1
    Ncurses.endwin
    puts "bye"
  end
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Nicolas Pouillard)
Date: Thu, 06 Aug 2009 20:09:35 +0200

> > what makes the difference. Sigh...
[...]
> If everyone's happy, I'll merge this branch into next.

Works just great! Thanks.

---

From: [email-masked] (Adeodato Simó)
Date: Thu, 6 Aug 2009 20:37:25 +0200

> into next. Let me know if it works for you.

Everything seems to work fine as of 3478e40, including sup terminating
upon SIGTERM. If I may, I'd suggest normal termination instead of
raising an exception. IMHO, it is expected that programs that capture
SIGTERM, will do cleanup and exit normally, rather than behave as if an
error had happened. (Perhaps it'd be difficult to exit sup cleanly from
the handler in any other way than an exception: that, I don't know.)

Thanks, in any case.

---

From: [email-masked] (Ben Walton)
Date: Thu, 06 Aug 2009 19:56:51 -0400

> in ruby ncurses libraries since at least 2005, when I complained about

...yup, looks like you stuck the landing this time! :)

Thanks
-Ben

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20090806/72c6f045/attachment.bin>

---

From: [email-masked] (William Morgan)
Date: Thu, 06 Aug 2009 18:38:53 -0700

Reformatted excerpts from Adeodato Sim?'s message of 2009-08-06:
> SIGTERM, will do cleanup and exit normally, rather than behave as if
> an error had happened.

Good idea. Try it now.

---

From: [email-masked] (Adeodato Simó)
Date: Fri, 7 Aug 2009 12:31:51 +0200

> Good idea. Try it now.

Ah, excellent: works as suggested now.

Regarding 57dea7a (refactor index locking interaction and replace
suicidemanager), I'll note that at least on my system, and after having
moved to Xapian, those codepaths no longer execute. That is, if a sup is
running and a second one is started, it never gets to ask whether to
suggest dieing to the first one, because this second one dies with:

[Fri Aug 07 12:31:05 +0200 2009] using character set encoding "UTF-8"
[Fri Aug 07 12:31:05 +0200 2009] using index Redwood::XapianIndex
[Fri Aug 07 12:31:05 +0200 2009] dynamically loading setlocale() from libc.so.6
[Fri Aug 07 12:31:05 +0200 2009] setting locale...
.../lib/sup/xapian_index.rb:24:in `initialize': Resource temporarily unavailable - /home/adeodato/.sup/entries.db (Errno::EAGAIN)
        from .../lib/sup/xapian_index.rb:24:in `new'
        from .../lib/sup/xapian_index.rb:24:in `initialize'
        from .../bin/sup:132:in `new'
        from .../bin/sup:132

Thanks!

---

From: [email-masked] (Rich Lane)
Date: Fri, 07 Aug 2009 12:40:18 -0400

Excerpts from Adeodato Sim?'s message of Fri Aug 07 06:31:51 -0400 2009:
> .../lib/sup/xapian_index.rb:24:in `initialize': Resource temporarily
> unavailable - /home/adeodato/.sup/entries.db (Errno::EAGAIN)

I've just posted a patch that should fix this.

---

From: [email-masked] (Adeodato Simó)
Date: Sat, 8 Aug 2009 11:23:25 +0200

> I've just posted a patch that should fix this.

It works, thank you!