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

socket closes prematurely #18227

Closed
bjarthur opened this issue Aug 24, 2016 · 13 comments
Closed

socket closes prematurely #18227

bjarthur opened this issue Aug 24, 2016 · 13 comments
Labels
needs docs Documentation for this change is required parallelism Parallel or distributed computation

Comments

@bjarthur
Copy link
Contributor

1a. first, listen, accept, readline, and println on the server-session:

julia> server = listen(2000)
Base.TCPServer(active)

julia> while true
            sock = accept(server)
            @async begin
                  while isopen(sock) || nb_available(sock)>0
                       tmp = chomp(readline(sock))
                       length(tmp)==0 && continue
                       println(tmp)
                  end
            end
      end
Task (runnable) @0x00007fdab1abcc40

2b. then, connect and println on client #1:

julia> sock = connect(2000)
TCPSocket(open, 0 bytes waiting)

julia> println(sock,"1")

3c. similarly, on client #2:

julia> sock = connect(2000)
TCPSocket(open, 0 bytes waiting)

julia> println(sock,"2")

4d. then println again back on client #1:

julia> println(sock,"1")

5e. ditto on client #2:

julia> println(sock,"2")

6f. now close client #2:

julia> close(sock)

7g. then back to client #1, and println no longer works:

julia> println(sock,"1")

note that if steps 4d and 5e are skipped, 7g works. also, client #2 still works if #1 is closed.

this in on 0.5.0-rc3 and scientific linux 7.

@bjarthur
Copy link
Contributor Author

@JeffBezanson
Copy link
Member

This is because every time around the while loop the same sock variable is overwritten, changing the value of sock in all of the tasks. You can create a new variable for each iteration by writing let sock = accept(server) ... end instead.

@vtjnash vtjnash closed this as completed Aug 24, 2016
@bjarthur
Copy link
Contributor Author

oi, thanks, sorry for the hash.

however, does @async no longer "wrap the expression in a let x=x, y=y, ... block to create a new scope with copies of all variables referenced in the expression" ? the above example works in 0.4, so something has changed.

@JeffBezanson
Copy link
Member

Hmm, good point. That does appear to be unintentional, but I actually think it's a good idea to remove the implicit let those docs are talking about --- less magical.

@tkelman tkelman added the needs docs Documentation for this change is required label Aug 24, 2016
@tkelman
Copy link
Contributor

tkelman commented Aug 24, 2016

reopen to fix the docs?

@bjarthur
Copy link
Contributor Author

would need to add it to NEWS.md, the docstring, and the example in Networking and Streams

@tkelman tkelman reopened this Aug 25, 2016
@bjarthur
Copy link
Contributor Author

is there a consensus to leave intact this unintentional change? is so, i can work on a PR to document it.

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

I actually think it's a good idea to remove the implicit let

Sounds like Jeff thinks so?

@bjarthur
Copy link
Contributor Author

bjarthur commented Sep 3, 2016

@fetchfrom also uses a magical closure. should we get rid of that too?

@ViralBShah ViralBShah added the parallelism Parallel or distributed computation label Sep 3, 2016
@ViralBShah
Copy link
Member

cc @amitmurthy

@bjarthur
Copy link
Contributor Author

@amitmurthy can you please comment on this change to the @async API? would be nice to have it addressed before the 0.6 feature freeze at the end of the year. thanks.

@amitmurthy
Copy link
Contributor

Same as #19468

I am fine with removing the implicit let. However, it is not just a doc change as currently a bug is preventing automatic localization of variables. The fix should

  • remove localize_vars from all exported macros/functions using it.
  • change docs
  • add a NEWS item for the changed behavior marking it as a breaking change.

@bjarthur
Copy link
Contributor Author

closed by #19594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs docs Documentation for this change is required parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants