Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(695)

Issue 8437090: Change the handling of closing sockets (Closed)

Created:
9 years, 1 month ago by Søren Gjesse
Modified:
9 years, 1 month ago
Reviewers:
Mads Ager (google)
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Change the handling of closing sockets Sockets now supports being half-closed for either reading or writing. When a socket is closed it is by default closed for both read and write and the underlying file descriptor is destroyed. However the socket close can be asked to only half-close the socket to send end of stream to the other end and still have the ability to receive more data. The close event on a socket is only emitted when the socket is closed by the other end. Both half-close and full-close by the other end is reported as a close event. If a socket is already half closed the close event will automatically destroy the socket. The streams on the sockets also takes advantage of this. A socket input stream will report a close event when the other end closed. A socket output stream will half-close the socket when close is called making it possible to still receive data on the input stream. When both streams have been closed the socket is destroyed. For sockets operating on pipes they are initially created as half-closed for either reading or writing depending on which type of pipe a socket object is based on. A pipe for writing will start half-closed for reading so if it is only half-closed for writing the socket will still be destroyed. Same with a pipe for reading that will start half-closed for writing and when a close event is received half-closing the other direction will destroy the socket. Socket objects based on pipes are only exposed through streams. Extended the socket close test to test a number of different scenarios. Also refactor the socket data C++ object to encapsulate more socket information and operations. Now the Linux and Mac OS versions are 100% the same as it turned out that using POLLRDHUP on Linux was not required any more. R=ager@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=1196

Patch Set 1 #

Patch Set 2 : Fix SocketCloseTest #

Patch Set 3 : Another fix to SocketCloseTest #

Total comments: 10

Patch Set 4 : Addressed review comments from ager@ #

Patch Set 5 : Rebase #

Patch Set 6 : Windows port and frog patch #

Total comments: 2

Patch Set 7 : Addressed review comments by ager@ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+746 lines, -342 lines) Patch
M frog/file_system_vm.dart View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/eventhandler.h View 1 chunk +2 lines, -0 lines 0 comments Download
M runtime/bin/eventhandler_linux.h View 1 2 3 4 chunks +54 lines, -13 lines 0 comments Download
M runtime/bin/eventhandler_linux.cc View 8 chunks +97 lines, -68 lines 0 comments Download
M runtime/bin/eventhandler_macos.h View 1 2 3 4 chunks +54 lines, -13 lines 0 comments Download
M runtime/bin/eventhandler_macos.cc View 8 chunks +97 lines, -66 lines 0 comments Download
M runtime/bin/eventhandler_win.h View 1 2 3 4 5 3 chunks +17 lines, -2 lines 0 comments Download
M runtime/bin/eventhandler_win.cc View 1 2 3 4 5 6 7 chunks +48 lines, -14 lines 0 comments Download
M runtime/bin/input_stream.dart View 1 chunk +6 lines, -7 lines 0 comments Download
M runtime/bin/output_stream.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M runtime/bin/process_impl.dart View 1 chunk +3 lines, -3 lines 0 comments Download
M runtime/bin/socket.dart View 1 2 3 2 chunks +14 lines, -9 lines 0 comments Download
M runtime/bin/socket_impl.dart View 1 2 3 11 chunks +65 lines, -16 lines 0 comments Download
M runtime/bin/socket_stream.dart View 7 chunks +23 lines, -19 lines 0 comments Download
M tests/standalone/src/EchoServerStreamTest.dart View 5 chunks +10 lines, -14 lines 0 comments Download
M tests/standalone/src/EchoServerTest.dart View 1 2 3 4 7 chunks +7 lines, -13 lines 0 comments Download
M tests/standalone/src/ProcessStderrTest.dart View 2 chunks +11 lines, -8 lines 0 comments Download
M tests/standalone/src/ProcessStdoutTest.dart View 2 chunks +3 lines, -5 lines 0 comments Download
M tests/standalone/src/SocketCloseTest.dart View 1 2 3 7 chunks +213 lines, -42 lines 0 comments Download
M tests/stub-generator/src/MintMakerFullyIsolatedTest-generatedTest.dart View 1 2 3 10 chunks +13 lines, -19 lines 0 comments Download
M tests/stub-generator/src/MintMakerPromiseWithStubsTest-generatedTest.dart View 1 2 3 5 chunks +6 lines, -8 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Søren Gjesse
This is Linux and Mac OS for now. I will update the cl with the ...
9 years, 1 month ago (2011-11-03 17:55:06 UTC) #1
Mads Ager (google)
LGTM http://codereview.chromium.org/8437090/diff/2017/runtime/bin/eventhandler_linux.h File runtime/bin/eventhandler_linux.h (right): http://codereview.chromium.org/8437090/diff/2017/runtime/bin/eventhandler_linux.h#newcode33 runtime/bin/eventhandler_linux.h:33: void ShutdownRead() { Add a blank line between ...
9 years, 1 month ago (2011-11-04 10:19:49 UTC) #2
Søren Gjesse
Added Windows port - PTAL. http://codereview.chromium.org/8437090/diff/2017/runtime/bin/eventhandler_linux.h File runtime/bin/eventhandler_linux.h (right): http://codereview.chromium.org/8437090/diff/2017/runtime/bin/eventhandler_linux.h#newcode33 runtime/bin/eventhandler_linux.h:33: void ShutdownRead() { On ...
9 years, 1 month ago (2011-11-04 12:15:11 UTC) #3
Mads Ager (google)
lgtm http://codereview.chromium.org/8437090/diff/8020/runtime/bin/eventhandler_win.cc File runtime/bin/eventhandler_win.cc (right): http://codereview.chromium.org/8437090/diff/8020/runtime/bin/eventhandler_win.cc#newcode350 runtime/bin/eventhandler_win.cc:350: reinterpret_cast<char *>(&s), sizeof(s)); char * -> char*
9 years, 1 month ago (2011-11-04 12:21:36 UTC) #4
Søren Gjesse
9 years, 1 month ago (2011-11-04 12:33:02 UTC) #5
http://codereview.chromium.org/8437090/diff/8020/runtime/bin/eventhandler_win.cc
File runtime/bin/eventhandler_win.cc (right):

http://codereview.chromium.org/8437090/diff/8020/runtime/bin/eventhandler_win...
runtime/bin/eventhandler_win.cc:350: reinterpret_cast<char *>(&s), sizeof(s));
On 2011/11/04 12:21:36, Mads Ager wrote:
> char * -> char*

Done.

Powered by Google App Engine
This is Rietveld 408576698