cleahcim / go9p

Automatically exported from code.google.com/p/go9p
Other
0 stars 0 forks source link

http package breaks multiple clients in one program #22

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
# What steps will reproduce the problem?

Trying to create two different clients with mount causes a panic, due to a 
double registration in the http package.

I've attached a test program that exhibits the problem, along with it's output 
to stderr (nothing comes out on stdout). The original code that caused me to 
notice the issue is here:

github.com/zenhack/cow9p

commit 78f0796f8e4f (which is current at the time of writing) causes the 
problem.

# What is the expected output? What do you see instead?

Should successfully mount both filesystems, and exit normally without producing 
any output. Instead, the http package causes a panic due to double registration 
(see stderr.log)

What version of the product are you using? On what operating system?

hg revison 120, gc compiler (go9p was built with go get) on linux amd64.

# Please provide any additional information below.

It seems like the http package shouldn't even need to be pulled in unless 
explicity asked for; I was a bit confused when I got a panic from a package I 
didn't think I was using. Could the http stuff go in a sub-package?

Original issue reported on code.google.com by i...@zenhack.net on 30 Dec 2012 at 3:23

Attachments:

GoogleCodeExporter commented 9 years ago
The http handling in go9p is broken and needs to be rethought. It was written 
when the http handler accepted 'nil' funcs and allowed things to be 
unregistered (now it panics). serve_http will have to be rewritten to do that 
instead of relying on package http.

Original comment by mirtchov...@gmail.com on 10 Jan 2013 at 7:30

GoogleCodeExporter commented 9 years ago
Perhaps the right thing to do is simply rip out the http stuff from clnt for 
now? linking the http package into anything that wants a 9p client smells like 
feature creep to me, and I managed to hit this bug without even knowing the 
http stuff was there.

Thoughts?

Original comment by i...@zenhack.net on 11 Jan 2013 at 11:12

GoogleCodeExporter commented 9 years ago

Original comment by lion...@gmail.com on 12 Jan 2013 at 5:44

GoogleCodeExporter commented 9 years ago
Fixed in two ways: fixed the original bug, http doesn't get included by default.

Original comment by lion...@gmail.com on 12 Jan 2013 at 6:02

GoogleCodeExporter commented 9 years ago
i believe the fundamental bug that you can't call http.HandleFunc with a nil 
func (i.e., the library panics if you unregister a client) is still there. 
we'll have to do our own basic mux-ing replacing DefaultServerMux which does 
the panic-ing.

Original comment by mirtchov...@gmail.com on 12 Jan 2013 at 6:11