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

Initialise DEPOT_PATH and LOAD_PATH before stdio #29252

Closed
wants to merge 1 commit into from
Closed

Initialise DEPOT_PATH and LOAD_PATH before stdio #29252

wants to merge 1 commit into from

Conversation

jameshclrk
Copy link

If stdin is a TCP socket, init_stdio tries to include the Sockets
module, but this is in stdlib and it fails to find the package if the
LOAD_PATH is not initialised.

#29234

If stdin is a TCP socket, init_stdio tries to include the Sockets
module, but this is in stdlib and it fails to find the package if the
LOAD_PATH is not initialised.

#29234
@vtjnash
Copy link
Member

vtjnash commented Sep 18, 2018

I don't think this is quite right. I think the problem is that we're trying to resolve Sockets relative to Base, but we actually just intending to grab a reference to the existing module. We do something similar elsewhere in the code as well (to load InteractiveUtils in client.jl).

diff --git a/base/stream.jl b/base/stream.jl
index 60834390a9..6e9e83622d 100644
--- a/base/stream.jl
+++ b/base/stream.jl
@@ -216,8 +216,6 @@ unlock(s::LibuvStream) = unlock(s.lock)
 rawhandle(stream::LibuvStream) = stream.handle
 unsafe_convert(::Type{Ptr{Cvoid}}, s::Union{LibuvStream, LibuvServer}) = s.handle
 
-const Sockets_mod = Ref{Module}()
-
 function init_stdio(handle::Ptr{Cvoid})
     t = ccall(:jl_uv_handle_type, Int32, (Ptr{Cvoid},), handle)
     if t == UV_FILE
@@ -233,10 +231,8 @@ function init_stdio(handle::Ptr{Cvoid})
     elseif t == UV_TTY
         return TTY(handle, StatusOpen)
     elseif t == UV_TCP
-        if !isassigned(Sockets_mod)
-            Sockets_mod[] = Base.require(Base, :Sockets)
-        end
-        return Sockets_mod[].TCPSocket(handle, StatusOpen)
+        Sockets = require(PkgId(UUID(0x6462fe0b_24de_5631_8697_dd941f90decc), "Sockets"))
+        return Sockets.TCPSocket(handle, StatusOpen)
     elseif t == UV_NAMED_PIPE
         return PipeEndpoint(handle, StatusOpen)
     else

(not tested)

@jameshclrk
Copy link
Author

That works! If it's used elsewhere in the code, that seems like a better solution.

@JeffBezanson
Copy link
Member

Ok, we should probably change this everywhere then. Looking for uses of require should find them.

@bjarthur
Copy link
Contributor

bump. ClusterManagers is broken on LSF because of this Socket issue i believe. can we please merge and backport?

@vchuravy
Copy link
Member

I adopted Jameson's suggestion in #29770

@vchuravy vchuravy closed this Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants