Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/rpc: gob encoding error not returned in response #7689

Closed
gopherbot opened this issue Apr 2, 2014 · 9 comments
Closed

net/rpc: gob encoding error not returned in response #7689

gopherbot opened this issue Apr 2, 2014 · 9 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@gopherbot
Copy link

by charlieandrews.cwa:

I was working with the "net/rpc" package using the default gob client/server
codec. Requests were being sent fine and all responses were being returned fine, except
for one, which would just hang. I could see that everything was going fine until it went
to write the response. I traced the issue all the way to the
gobServerCodec.WriteResponse function in "net/rpc/server.go". I put some log
lines in, and noticed that it encoded the Response just fine, but errored on the
encoding of the body. It seems that there was no way for me to know it errored on the
body unless I had debugLog set to true (which there doesn't seem to be a nice way to
do), or put a log line in there. Is there a way that the order that the Response and
body get encoded could be switched, so that if an error occurs while encoding the body,
it will be reported in Response.Error ? I think the change would be trivial, so I will
do it if others think that I am correct in thinking that this is a bug.
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.3maybe.

@gopherbot
Copy link
Author

Comment 2 by charlieandrews.cwa:

I posted yesterday on the mailing list with a proposed change.
https://groups.google.com/forum/#!topic/golang-nuts/AMojGgap0YQ

@nigeltao
Copy link
Contributor

nigeltao commented Apr 9, 2014

Comment 3:

Owner changed to @robpike.

@rsc
Copy link
Contributor

rsc commented May 11, 2014

Comment 4:

I looked into this briefly. If you buffer the response you end up with an unnecessary
second copy in memory. Maybe it's not a big deal, I don't know.
At the least perhaps gobServerCodec.WriteResponse should detect the lack of I/O error
and do something like print a log message, shut down the connection, or emit a gob value
that will cause a decoding error on the other side.
    if err = c.enc.Encode(r); err != nil {
        if c.encBuf.Flush() == nil {
            // was a gob problem, not a write problem. do something
        }
        return
    }
    if err = c.enc.Encode(body); err != nil {
        if c.encBuf.Flush() == nil {
            // was a gob problem, not a write problem. do something
        }
        return
    }
    return c.encBuf.Flush()

Status changed to Accepted.

@robpike
Copy link
Contributor

robpike commented May 12, 2014

Comment 5:

Labels changed: added release-go1.4, removed release-go1.3maybe.

@gopherbot
Copy link
Author

Comment 6:

CL https://golang.org/cl/146670043 mentions this issue.

@robpike
Copy link
Contributor

robpike commented Oct 1, 2014

Comment 7:

This issue was closed by revision cd5b785.

Status changed to Fixed.

@robpike
Copy link
Contributor

robpike commented Oct 3, 2014

Comment 8:

Issue #8173 has been merged into this issue.

@gopherbot
Copy link
Author

Comment 9:

CL https://golang.org/cl/151370043 mentions this issue.

@robpike robpike mentioned this issue Dec 8, 2014
Closed
@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@robpike @wheatman
net/rpc: shut down connection if gob has error
0d6a8c8 
The nicest solution would be to buffer the message and only write
it if it encodes correctly, but that adds considerable memory and
CPU overhead for a very rare condition. Instead, we just shut
down the connection if this happens.
Fixes golang#7689.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/146670043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@robpike @wheatman
net/rpc: shut down connection if gob has error
3177ea9 
The nicest solution would be to buffer the message and only write
it if it encodes correctly, but that adds considerable memory and
CPU overhead for a very rare condition. Instead, we just shut
down the connection if this happens.
Fixes golang#7689.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/146670043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@robpike @wheatman
net/rpc: shut down connection if gob has error
084f031 
The nicest solution would be to buffer the message and only write
it if it encodes correctly, but that adds considerable memory and
CPU overhead for a very rare condition. Instead, we just shut
down the connection if this happens.
Fixes golang#7689.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/146670043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@robpike @wheatman
net/rpc: shut down connection if gob has error
8c45539 
The nicest solution would be to buffer the message and only write
it if it encodes correctly, but that adds considerable memory and
CPU overhead for a very rare condition. Instead, we just shut
down the connection if this happens.
Fixes golang#7689.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/146670043
@rsc rsc unassigned robpike Jun 23, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4
Development

No branches or pull requests
5 participants
@rsc @ianlancetaylor @robpike @nigeltao @gopherbot