Re: The Links/Links2/ELinks browsers are unusable on Debian GNU/Hurd

On Thu, Dec 20, 2007 at 06:29:51AM +0200, Kalle Olavi Niemitalo wrote:
> > 114		if (!len) return;
> > (gdb) n
> > 116		if (!itrm->out.queue.len && can_write(itrm->out.sock)) {
> > (gdb) n
> > 119				register_bottom_half(free_itrm, itrm);
> > (gdb) n
> > 124		if (w < len) {
> 
> Recompile without optimization to see what really happens here.

(gdb) run -no-home
Starting program: /home/fabi/hurd/elinks/src/elinks -no-home

Breakpoint 1, itrm_queue_event (itrm=0x81a1f40, data=0x1020b18 "", len=320)
    at kbd.c:112
112		int w = 0;
(gdb) n
114		if (!len) return;
(gdb) n
116		if (!itrm->out.queue.len && can_write(itrm->out.sock)) {
(gdb) n
124		if (w < len) {
(gdb) n
125			int left = len - w;
(gdb) n
126			unsigned char *c = mem_realloc(itrm->out.queue.data,
(gdb) n
129			if (!c) {
(gdb) n
134			itrm->out.queue.data = c;
(gdb) n
135			memcpy(itrm->out.queue.data + itrm->out.queue.len, data + w, left);
(gdb) n
136			itrm->out.queue.len += left;
(gdb) n
137			set_handlers(itrm->out.sock,
(gdb) n
142	}
(gdb) n
handle_trm (std_in=0, std_out=1, sock_in=1, sock_out=7, ctl_in=0, 
    init_string=0x81a1dd0, init_len=0, remote=0) at kbd.c:345
345		itrm_queue_event(itrm, (char *) init_string, init_len);

    The test line 116 fails here and not on Linux.  I still don't
understand why gdb says that it goes to the line 119
(check_bottom_halves) with optimization?
    With a breakpoint at can_read_or_write:

(gdb) run -no-home
Starting program: /home/fabi/hurd/elinks/src/elinks -no-home

Breakpoint 1, itrm_queue_event (itrm=0x81a1f40, data=0x1020b18 "", len=320)
    at kbd.c:112
112		int w = 0;
(gdb) n
114		if (!len) return;
(gdb) n
116		if (!itrm->out.queue.len && can_write(itrm->out.sock)) {
(gdb) print *itrm
$1 = {in = {std = 0, sock = 1, ctl = 0, queue = {data = 0x81a1fd0 "", 
      len = 0}}, out = {std = 1, sock = 7, queue = {data = 0x0, len = 0}}, 
  timer = 0x0, t = {c_iflag = 10240, c_oflag = 3, c_cflag = 19201, 
    c_lflag = 4194499, c_cc = "\004ÿÿ\177\027\025\022ÿ\003ÿÿÿÿÿÿÿ\001\000ÿÿ", 
    __ispeed = 38400, __ospeed = 38400}, mouse_h = 0x0, orig_title = 0x0, 
  blocked = 0, altscreen = 1, touched_title = 0, remote = 0}
(gdb) n

Breakpoint 2, can_read_or_write (fd=7, write=1) at select.c:319
319		struct timeval tv = {0, 0};
(gdb) n
321		fd_set *rfds = NULL;
(gdb) n
322		fd_set *wfds = NULL;
(gdb) n
324		FD_ZERO(&fds);
(gdb) n
325		FD_SET(fd, &fds);
(gdb) n
327		if (write)
(gdb) n
328			wfds = &fds;
(gdb) n
332		return select(fd + 1, rfds, wfds, NULL, &tv);
(gdb) print wfds
$2 = (fd_set *) 0x1020a6c
(gdb) print rfds
$3 = (fd_set *) 0x0
(gdb) print &tv
$4 = (struct timeval *) 0x1020a8c
(gdb) print select(8,0x0,0x1020a6c,0x0,0x1020a8c)
$5 = 0
(gdb) n
333	}
(gdb) n
can_write (fd=7) at select.c:345
345	}
(gdb) n
itrm_queue_event (itrm=0x81a1f40, data=0x1020b18 "", len=320) at kbd.c:124
124		if (w < len) {
(gdb) n
125			int left = len - w;

> I mean, the compiler might merely have decided to load the
> address of free_itrm into a register at that point, or something
> like that.

    But how could I see that?

> > As you can see, there is something wrong there,
> 
> I don't see that.

    You are probably more experienced than me in that sort of thing, for
me that seems at least very weird. :-)

> So the patch doesn't reveal anything.

   It's true, I had not observed enough what it does.

> Instead, you should find out why the select function is reporting
> an exceptional state for the file descriptor.

   According to the gdb output above select in can_read_or_write(7,1)
returns 0 whereas on Linux it returns 1 in the same conditions, but why,
I don't know.

> I suggest you use rpctrace to find whether any io_select is
> returning the SELECT_URG flag in select_type, and from which
> server that reply comes. 

    I tried rpctrace elinks but I don't know what to look for in the
output...  There are lines such as:

task2573->vm_deallocate (21331968 4096) = 0 
  88->io_select_request (2)task2573->mach_port_destroy (pn{ 25}) = 0 
reply(94:io_select_request)->io_select_reply (0 2);
  72->io_write_request ("^[[1;1H" -1) = 0 6
  52->io_select_request (5)task2573->mach_port_allocate (3) = 0 pn{ 29}
task2573->mach_port_move_member (pn{ 25} pn{ 29}) = 0 
  86->io_select_request (5)task2573->mach_port_move_member (pn{ 27} pn{ 29}) = 0 
  88->io_select_request (6)task2573->mach_port_move_member (pn{ 28} pn{ 29}) = 0 
reply(96:io_select_request)->io_select_reply (0 2);
task2573->mach_port_destroy (pn{ 25}) = 0 

    What must I look here?  the numbers between parenthesis after
io_select_reply (in that case, they are always (0 1) or (0 2) or (0 0))?
something completely different?

> Also try putting the NULLs in ELinks
> like I mentioned in the previous message.

Something like the following patch?  It seems to work, but perhaps it
breaks something else.

Fabienne.

diff --git a/src/main/select.c b/src/main/select.c
index aab5349..7311c6f 100644
--- a/src/main/select.c
+++ b/src/main/select.c
@@ -33,6 +33,7 @@
 #include "main/select.h"
 #include "main/timer.h"
 #include "osdep/signals.h"
+#include "terminal/kbd.h"
 #include "terminal/terminal.h"
 #include "util/error.h"
 #include "util/memory.h"
@@ -148,7 +149,9 @@ set_handlers(int fd, select_handler_T read_func, select_handler_T write_func,
 #endif
 	threads[fd].read_func = read_func;
 	threads[fd].write_func = write_func;
-	threads[fd].error_func = error_func;
+	threads[fd].error_func = NULL;
+	if (error_func != (select_handler_T) free_itrm)
+		threads[fd].error_func = error_func;
 	threads[fd].data = data;
 
 	if (read_func) {
diff --git a/src/terminal/kbd.c b/src/terminal/kbd.c
index 1d34232..c2ff503 100644
--- a/src/terminal/kbd.c
+++ b/src/terminal/kbd.c
@@ -43,7 +43,6 @@
 
 struct itrm *ditrm = NULL;
 
-static void free_itrm(struct itrm *);
 static void in_kbd(struct itrm *);
 static void in_sock(struct itrm *);
 static int process_queue(struct itrm *);
@@ -397,7 +396,7 @@ block_itrm(void)
 }
 
 
-static void
+void
 free_itrm(struct itrm *itrm)
 {
 	if (!itrm) return;
diff --git a/src/terminal/kbd.h b/src/terminal/kbd.h
index e886f8c..bf95976 100644
--- a/src/terminal/kbd.h
+++ b/src/terminal/kbd.h
@@ -121,6 +121,7 @@ void itrm_queue_event(struct itrm *itrm, unsigned char *data, int len);
 void block_itrm(void);
 int unblock_itrm(void);
 void free_all_itrms(void);
+void free_itrm(struct itrm *);
 void resize_terminal(void);
 void dispatch_special(unsigned char *);
 void kbd_ctrl_c(void);