#s doesn't reply on pending Tread/Twrite

[GoogleCodeExporter]

What steps will reproduce the problem?
% ps | grep Cat
% ls '#sbug'
% srvbug &
% ls '#sbug'
#sbug/data
% cat '#sbug/data' &
% got read request

% ls '#sbug'
% ps | grep Cat
     157       57   powerman    0:00.0    release    74K Cat[$Sys]

What is the expected output? What do you see instead?
I expect 'cat' process to exit when 'srvbug' process exited and '#s' driver 
automatically removed '#sbug/data'.

Which operating system are you using?
Linux x86, emu version:
changeset:   466:17c8d0a73ef8
date:        Sat Nov 27 16:50:34 2010 +0000

Please provide any additional information below.
---cut srvbug.b:
init(nil: ref Draw->Context, nil: list of string)
{
        sys = load Sys Sys->PATH;
        fio := sys->file2chan("#sbug", "data");
        <-fio.read;
        sys->print("got read request\n");
}

Original issue reported on code.google.com by powerman...@gmail.com on 9 Dec 2010 at 3:19

[GoogleCodeExporter]

I've found this issue already documented on srv(3), so this report probably 
useless.
To make it a little more useful, I've attached patch which fix this issue.
There should be no memory leaks, but I'm not sure is all locking done correctly.

Anyway, now it works as expected:
; srvbug &
; cat '#sbug/data' &
; got read request
cat: error reading #sbug/data: i/o on hungup channel

; ps | grep Cat
;

Original comment by powerman...@gmail.com on 11 Dec 2010 at 12:17

Attachments:

• issue-244-devsrv.patch

[GoogleCodeExporter]

fixed 2 memory leaks, one on error and another on uncaught exception

Original comment by powerman...@gmail.com on 11 Dec 2010 at 2:27

Attachments:

• issue-244-devsrv.patch

[GoogleCodeExporter]

fixed formatting (replaced few spaces with tab)

Original comment by powerman...@gmail.com on 11 Dec 2010 at 2:40

Attachments:

• issue-244-devsrv.patch

[GoogleCodeExporter]

fixed race

Original comment by powerman...@gmail.com on 11 Dec 2010 at 1:35

Attachments:

• issue-244-devsrv.patch

[GoogleCodeExporter]

man page patch

Original comment by powerman...@gmail.com on 11 Dec 2010 at 2:04

Attachments:

• issue-244-man3srv.patch

[GoogleCodeExporter]

Original comment by Charles....@gmail.com on 12 Dec 2010 at 12:17

• Changed state: Checking

[GoogleCodeExporter]

it's slightly embarrassing that roger peppe sent a similar fix (slightly 
simpler) some time ago, but it wasn't applied. since i can't remember why it 
wasn't applied, i shall study both fixes and apply one or a mixture.

Original comment by Charles....@gmail.com on 13 Dec 2010 at 1:44

• Changed state: Accepted

[GoogleCodeExporter]

Can you attach his patch here? I'd like to study it too, because I didn't see 
what can be simplified in that patch.

I see some things which can be improved in my patch:
1) I didn't really like chosen func/struct names, but fail to invent better 
names
2) pend.chanlist = nil; in srvinit() is probably redundant
3) both "reply" struct can be moved to constant values instead of generating 
them each time, but I'm not sure is this optimization actually needed

But that's not simplifying. And simplifying something else may lead to creating 
memory leaks or race condition bugs. Probably single flat linked list can be 
used instead of list of lists, but that can harm performance on high load (I 
found myself using file2chan a lot because it's much simpler than full 9P 
server and handling only read/write is enough in most cases). But I agree this 
may be premature optimization right now.

Original comment by powerman...@gmail.com on 13 Dec 2010 at 2:46

[GoogleCodeExporter]

Oh, and I forgot about:
4) csendalt() may be used instead of csend() in penddelchan()

I'm not sure what csendalt() exactly does, but I suppose it's equivalent of 
limbo's non-blocking send: "alt{c<-=reply=>; *=>;}". But in all my tests it 
never block in that csend(), so I decided csendalt() doesn't needed.

Original comment by powerman...@gmail.com on 13 Dec 2010 at 2:54

[GoogleCodeExporter]

roger's stored the list of requests in the SrvFile itself, and (more subtly) 
avoided allocating descriptors on the heap, because any request must have the 
process blocked waiting in crecv, so the value describing the pending operation 
can be  on the stack.

the details were still not quite right. i've attached a revised version that 
seems all right (though i haven't tested it extensively yet).

Original comment by Charles....@gmail.com on 17 Dec 2010 at 10:22

• Changed state: Started

Attachments:

• devsrv.c

[GoogleCodeExporter]

Original comment by Charles....@gmail.com on 17 Dec 2010 at 10:23

• Changed state: Testing

[GoogleCodeExporter]

CHANGES
emu/port/devsrv.c
man/3/srv
os/port/devsrv.c
committed changeset 471:776d4fa0ae38

Original comment by Charles....@gmail.com on 20 Dec 2010 at 4:41

• Changed state: Verified

[GoogleCodeExporter]

I think calls to delwaiting() should be moved few lines down (after poperror()) 
to avoid calling it twice in case of error() on next line at:
http://code.google.com/p/inferno-os/source/browse/emu/port/devsrv.c#613
http://code.google.com/p/inferno-os/source/browse/emu/port/devsrv.c#709

Original comment by powerman...@gmail.com on 2 Apr 2011 at 7:42

[GoogleCodeExporter]

Same apply to os/port/devsrv.c, of course.

Original comment by powerman...@gmail.com on 2 Apr 2011 at 7:45

[GoogleCodeExporter]

the "waiting" state should also surround the crecvs closely

Original comment by Charles....@gmail.com on 2 Apr 2011 at 9:54

[GoogleCodeExporter]

committed changeset 502:98d226096414

Original comment by Charles....@gmail.com on 2 Apr 2011 at 10:51