close(s::Session) waits forever 💤

samoconnor commented 10 years ago

close(s::Session) calls gnutls_bye(s.handle, GNUTLS_SHUT_RDWR). This causes gnutls to "wait for the peer to reply with the same message" -- http://dev.man-online.org/man3/gnutls_bye

I'm using Requests.jl to talk to https://iam.amazonaws.com. gnutls_bye hangs forever.

The patch below makes it work. I don't really know enough about how GnuTLS.jl is intended to work to be sure this fix is a good one, or if AWS is "doing the wrong thing". However, I think it is fair to say that being compatible with AWS is a useful thing.

My code calling Requests.jl is here: https://github.com/samoconnor/OCAWS.jl/blob/master/src/http.jl#L42

--- a/src/GnuTLS.jl
+++ b/src/GnuTLS.jl
@@ -88,12 +88,12 @@ const GNUTLS_SHUT_WR = 1

 free_session(s::Session) = ccall((:gnutls_deinit,gnutls),Void,(Ptr{Void},),s.handle)
-isopen(s::Session) = (isopen(s.read) || isopen(s.write))
+isopen(s::Session) = (s.open && (isopen(s.read) || isopen(s.write)))

 function close(s::Session)
        ret::Int32 = 0
        try # The remote might very well simply shut the stream rather than acknowledge the closure
-               ret = ccall((:gnutls_bye,gnutls), Int32, (Ptr{Void},Int32), s.handle, GNUTLS_SHUT_RDWR)
+               ret = ccall((:gnutls_bye,gnutls), Int32, (Ptr{Void},Int32), s.handle, GNUTLS_SHUT_WR)
        catch e
                if !isa(e,EOFError)
                        rethrow()

samoconnor commented 9 years ago

Any ideas? Is this library being maintained?

Keno commented 9 years ago

Is the underlying transport being terminated or does AWS just leave it open?

samoconnor commented 9 years ago

I'm afraid I don't recall the detail. However, it seems this problem has come up before elsewhere:

https://github.com/openldap/openldap/commit/6ad808dc1db41fabf29a80842649f1105782f890

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=386530#5

https://gitorious.org/libsoup-elima/libsoup-elima/commit/ef972798309087651a4976d02f02510b995a527a

samoconnor commented 9 years ago

bump...

sbromberger commented 9 years ago

@samoconnor this may be related to https://github.com/JuliaWeb/Requests.jl/issues/42 - in any case, I think it's safe to say that GnuTLS is not in a stable state at the moment.

sbromberger commented 9 years ago

@samoconnor For what it's worth, your patch appears to fix https://github.com/JuliaWeb/Requests.jl/issues/42. This looks promising. Can you do a Pkg.checkout("GnuTLS","sab/fixup1") and Pkg.checkout("Requests","sab/fixup") and let me know whether you're getting better results?

IainNZ commented 9 years ago

@sbromberger I'm happy to report that I was able to run PkgEval with no glitches. Maybe make a new PR for GnuTLS with your changes (rebased to a single commit?) and we'll try and get Travis green. Tag that, get Requests green, then tag Requests.

sbromberger commented 9 years ago

Merged #36. Will tag shortly.

sbromberger commented 9 years ago

Tagged. Will close this out. I implemented this specific change (GNUTLS_SHUT_RDWR to GNUTLS_SHUT_WR) in the PR.

JuliaWeb / GnuTLS.jl

close(s::Session) waits forever 💤 #21