From ac5c191683077e2eaeed24430db6ca58ee4d8a7e Mon Sep 17 00:00:00 2001 From: "ager@google.com" Date: Fri, 10 Feb 2012 11:00:15 +0000 Subject: [PATCH] Another attempt at fixing the Socket tests. The tests did not wait for the server isolate to finish before exiting the main isolate. The tests did not correctly count the number of bytes received on the server-side. We have to count bytes per connection. There were issues with close handlers being confused for timeouts. R=sgjesse@google.com BUG= TEST= Review URL: https://chromiumcodereview.appspot.com//9382001 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@4127 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/bin/socket_impl.dart | 39 ++-- runtime/bin/socket_stream_impl.dart | 10 +- tests/standalone/src/SocketCloseTest.dart | 112 +++++++---- .../standalone/src/SocketStreamCloseTest.dart | 190 +++++++++++++----- 4 files changed, 245 insertions(+), 106 deletions(-) diff --git a/runtime/bin/socket_impl.dart b/runtime/bin/socket_impl.dart index 6b5293601a2..a4822d09d95 100644 --- a/runtime/bin/socket_impl.dart +++ b/runtime/bin/socket_impl.dart @@ -58,7 +58,7 @@ class _SocketBase { // Don't call the in handler if there is no data available // after all. - if (i == _IN_EVENT && this is _Socket && available() == 0) { + if ((i == _IN_EVENT) && (this is _Socket) && (available() == 0)) { continue; } eventHandler(); @@ -132,28 +132,34 @@ class _SocketBase { } void _closeWrite() { - if (_closedRead) { - _close(); - } else { - _sendToEventHandler(1 << _SHUTDOWN_WRITE_COMMAND); + if (_id >= 0) { + if (_closedRead) { + _close(); + } else { + _sendToEventHandler(1 << _SHUTDOWN_WRITE_COMMAND); + } + _closedWrite = true; } - _closedWrite = true; } void _closeRead() { - if (_closedWrite) { - _close(); - } else { - _sendToEventHandler(1 << _SHUTDOWN_READ_COMMAND); + if (_id >= 0) { + if (_closedWrite) { + _close(); + } else { + _sendToEventHandler(1 << _SHUTDOWN_READ_COMMAND); + } + _closedRead = true; } - _closedRead = true; } void _close() { - _sendToEventHandler(1 << _CLOSE_COMMAND); - _handler.close(); - _handler = null; - _id = -1; + if (_id >= 0) { + _sendToEventHandler(1 << _CLOSE_COMMAND); + _handler.close(); + _handler = null; + _id = -1; + } } void _sendToEventHandler(int data) { @@ -161,6 +167,7 @@ class _SocketBase { _handler = new ReceivePort(); _handler.receive((var message, ignored) { _multiplex(message); }); } + assert(_id >= 0); _EventHandler._sendData(_id, _handler, data); } @@ -382,7 +389,7 @@ class _Socket extends _SocketBase implements Socket { void set closeHandler(void callback()) { if (_inputStream != null) throw new StreamException( - "Cannot set data handler when input stream is used"); + "Cannot set close handler when input stream is used"); _closeHandler = callback; } diff --git a/runtime/bin/socket_stream_impl.dart b/runtime/bin/socket_stream_impl.dart index d04c41bfb43..8a8ca8c57e4 100644 --- a/runtime/bin/socket_stream_impl.dart +++ b/runtime/bin/socket_stream_impl.dart @@ -4,6 +4,7 @@ class _SocketInputStream implements SocketInputStream { _SocketInputStream(Socket socket) : _socket = socket { + if (_socket._id == -1) _closed = true; _socket.closeHandler = _closeHandler; } @@ -51,7 +52,6 @@ class _SocketInputStream implements SocketInputStream { void close() { if (!_closed) { _socket.close(); - if (_clientCloseHandler !== null) _clientCloseHandler(); } } @@ -72,7 +72,9 @@ class _SocketInputStream implements SocketInputStream { void _closeHandler() { _closed = true; - if (_clientCloseHandler !== null) _clientCloseHandler(); + if (_clientCloseHandler !== null) { + _clientCloseHandler(); + } } Socket _socket; @@ -120,10 +122,6 @@ class _SocketOutputStream implements SocketOutputStream { } } - void set closeHandler(void callback()) { - _socket.closeHandler = callback; - } - void set errorHandler(void callback()) { _streamErrorHandler = callback; if (_streamErrorHandler != null) { diff --git a/tests/standalone/src/SocketCloseTest.dart b/tests/standalone/src/SocketCloseTest.dart index ea7daf36461..86e59ddeb98 100644 --- a/tests/standalone/src/SocketCloseTest.dart +++ b/tests/standalone/src/SocketCloseTest.dart @@ -2,6 +2,11 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. // +// VMOptions= +// VMOptions=--short_socket_read +// VMOptions=--short_socket_write +// VMOptions=--short_socket_read --short_socket_write +// // Test socket close events. #import("dart:io"); @@ -12,15 +17,14 @@ final ITERATIONS = 10; class SocketClose { - SocketClose.start(mode) + SocketClose.start(this._mode, this._donePort) : _receivePort = new ReceivePort(), _sendPort = null, _readBytes = 0, _dataEvents = 0, _closeEvents = 0, _errorEvents = 0, - _iterations = 0, - _mode = mode { + _iterations = 0 { new SocketCloseServer().spawn().then((SendPort port) { _sendPort = port; start(); @@ -48,8 +52,8 @@ class SocketClose { case 4: case 5: case 6: - List b = new List(100); - _readBytes += _socket.readList(b, 0, 100); + List b = new List(5); + _readBytes += _socket.readList(b, 0, 5); if ((_readBytes % 5) == 0) { _dataEvents++; } @@ -96,6 +100,15 @@ class SocketClose { _socket.closeHandler = closeHandler; _socket.errorHandler = errorHandler; + void writeHello() { + int bytesWritten = 0; + while (bytesWritten != 5) { + bytesWritten += _socket.writeList("Hello".charCodes(), + bytesWritten, + 5 - bytesWritten); + } + } + _iterations++; switch (_mode) { case 0: @@ -103,28 +116,23 @@ class SocketClose { proceed(); break; case 1: - int bytesWritten = _socket.writeList("Hello".charCodes(), 0, 5); - Expect.equals(5, bytesWritten); + writeHello(); _socket.close(); proceed(); break; case 2: case 3: - int bytesWritten = _socket.writeList("Hello".charCodes(), 0, 5); - Expect.equals(5, bytesWritten); + writeHello(); break; case 4: - int bytesWritten = _socket.writeList("Hello".charCodes(), 0, 5); - Expect.equals(5, bytesWritten); + writeHello(); _socket.close(true); break; case 5: - int bytesWritten = _socket.writeList("Hello".charCodes(), 0, 5); - Expect.equals(5, bytesWritten); + writeHello(); break; case 6: - int bytesWritten = _socket.writeList("Hello".charCodes(), 0, 5); - Expect.equals(5, bytesWritten); + writeHello(); _socket.close(true); break; default: @@ -147,7 +155,10 @@ class SocketClose { void shutdown() { _sendPort.send(SERVERSHUTDOWN, _receivePort.toSendPort()); - _receivePort.close(); + _receivePort.receive((message, ignore) { + _donePort.send(null); + _receivePort.close(); + }); switch (_mode) { case 0: @@ -183,8 +194,17 @@ class SocketClose { int _errorEvents; int _iterations; int _mode; + int _donePort; } + +class ConnectionData { + ConnectionData(Socket this.connection) : readBytes = 0; + Socket connection; + int readBytes; +} + + class SocketCloseServer extends Isolate { static final HOST = "127.0.0.1"; @@ -193,45 +213,58 @@ class SocketCloseServer extends Isolate { void main() { - void connectionHandler(Socket connection) { + void connectionHandler(ConnectionData data) { + var connection = data.connection; void readBytes(whenFiveBytes) { - List b = new List(100); - _readBytes += connection.readList(b, 0, 100); - if ((_readBytes % 5) == 0) { + List b = new List(5); + data.readBytes += connection.readList(b, 0, 5); + if (data.readBytes == 5) { whenFiveBytes(); } } + void writeHello() { + int bytesWritten = 0; + while (bytesWritten != 5) { + bytesWritten += connection.writeList("Hello".charCodes(), + bytesWritten, + 5 - bytesWritten); + } + } + void dataHandler() { - _dataEvents++; switch (_mode) { case 0: Expect.fail("No data expected"); break; case 1: - readBytes(() { }); + readBytes(() { _dataEvents++; }); break; case 2: readBytes(() { + _dataEvents++; connection.close(); }); break; case 3: readBytes(() { - connection.writeList("Hello".charCodes(), 0, 5); + _dataEvents++; + writeHello(); connection.close(); }); break; case 4: readBytes(() { - connection.writeList("Hello".charCodes(), 0, 5); + _dataEvents++; + writeHello(); }); break; case 5: case 6: readBytes(() { - connection.writeList("Hello".charCodes(), 0, 5); + _dataEvents++; + writeHello(); connection.close(true); }); break; @@ -264,9 +297,10 @@ class SocketCloseServer extends Isolate { // Make sure all iterations have been run. In multiple of these // scenarios it is possible to get the SERVERSHUTDOWN message // before we have received the last close event on the - // server. We therefore always wait for the correct number of + // server. In these cases we wait for the correct number of // close events. - if (_iterations == ITERATIONS && _closeEvents == ITERATIONS) { + if (_iterations == ITERATIONS && + (_closeEvents == ITERATIONS || (_mode == 2 || _mode == 3))) { switch (_mode) { case 0: Expect.equals(0, _dataEvents); @@ -293,12 +327,14 @@ class SocketCloseServer extends Isolate { Expect.equals(0, _errorEvents); _server.close(); this.port.close(); + _donePort.send(null); } else { new Timer(waitForResult, 100); } } this.port.receive((message, SendPort replyTo) { + _donePort = replyTo; if (message != SERVERSHUTDOWN) { _readBytes = 0; _errorEvents = 0; @@ -308,7 +344,10 @@ class SocketCloseServer extends Isolate { _mode = message; _server = new ServerSocket(HOST, 0, 10); Expect.equals(true, _server !== null); - _server.connectionHandler = connectionHandler; + _server.connectionHandler = (connection) { + var data = new ConnectionData(connection); + connectionHandler(data); + }; _server.errorHandler = errorHandlerServer; replyTo.send(_server.port, null); } else { @@ -318,6 +357,7 @@ class SocketCloseServer extends Isolate { } ServerSocket _server; + SendPort _donePort; int _readBytes; int _errorEvents; int _dataEvents; @@ -336,11 +376,13 @@ main() { // 4: Client sends and half-closes. Server responds and closes. // 5: Client sends. Server responds and half closes. // 6: Client sends and half-closes. Server responds and half closes. - new SocketClose.start(0); - new SocketClose.start(1); - new SocketClose.start(2); - new SocketClose.start(3); - new SocketClose.start(4); - new SocketClose.start(5); - new SocketClose.start(6); + var tests = 7; + var port = new ReceivePort(); + var completed = 0; + port.receive((message, ignore) { + if (++completed == tests) port.close(); + }); + for (var i = 0; i < tests; i++) { + new SocketClose.start(i, port.toSendPort()); + } } diff --git a/tests/standalone/src/SocketStreamCloseTest.dart b/tests/standalone/src/SocketStreamCloseTest.dart index ed788c5e420..23777247ad0 100644 --- a/tests/standalone/src/SocketStreamCloseTest.dart +++ b/tests/standalone/src/SocketStreamCloseTest.dart @@ -2,6 +2,11 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. // +// VMOptions= +// VMOptions=--short_socket_read +// VMOptions=--short_socket_write +// VMOptions=--short_socket_read --short_socket_write +// // Test socket close events. #import("dart:io"); @@ -12,14 +17,14 @@ final ITERATIONS = 10; class SocketClose { - SocketClose.start(mode) + SocketClose.start(this._mode, this._donePort) : _receivePort = new ReceivePort(), _sendPort = null, + _readBytes = 0, _dataEvents = 0, _closeEvents = 0, _errorEvents = 0, - _iterations = 0, - _mode = mode { + _iterations = 0 { new SocketCloseServer().spawn().then((SendPort port) { _sendPort = port; start(); @@ -47,9 +52,13 @@ class SocketClose { case 4: case 5: case 6: - List b = new List(100); - _socket.readList(b, 0, 100); - _dataEvents++; + case 7: + case 8: + var read = _socket.inputStream.read(); + _readBytes += read.length; + if ((_readBytes % 5) == 0) { + _dataEvents++; + } break; default: Expect.fail("Unknown test mode"); @@ -64,17 +73,19 @@ class SocketClose { break; case 2: case 3: - _socket.outputStream.close(); - proceed(); - break; case 4: + _socket.outputStream.close(); proceed(); break; case 5: - _socket.outputStream.close(); proceed(); break; case 6: + _socket.outputStream.close(); + proceed(); + break; + case 7: + case 8: proceed(); break; default: @@ -100,23 +111,31 @@ class SocketClose { break; case 1: _socket.outputStream.write("Hello".charCodes()); - _socket.inputStream.close(); - proceed(); + _socket.outputStream.noPendingWriteHandler = () { + _socket.inputStream.close(); + proceed(); + }; break; case 2: case 3: - _socket.outputStream.write("Hello".charCodes()); - break; case 4: _socket.outputStream.write("Hello".charCodes()); - _socket.outputStream.close(); break; case 5: _socket.outputStream.write("Hello".charCodes()); + _socket.outputStream.noPendingWriteHandler = () { + _socket.outputStream.close(); + }; break; case 6: _socket.outputStream.write("Hello".charCodes()); - _socket.outputStream.close(); + break; + case 7: + case 8: + _socket.outputStream.write("Hello".charCodes()); + _socket.outputStream.noPendingWriteHandler = () { + _socket.outputStream.close(); + }; break; default: Expect.fail("Unknown test mode"); @@ -138,22 +157,32 @@ class SocketClose { void shutdown() { _sendPort.send(SERVERSHUTDOWN, _receivePort.toSendPort()); - _receivePort.close(); + _receivePort.receive((message, ignore) { + _donePort.send(null); + _receivePort.close(); + }); switch (_mode) { case 0: case 1: Expect.equals(0, _dataEvents); - Expect.equals(10, _closeEvents); + Expect.equals(0, _closeEvents); break; case 2: Expect.equals(0, _dataEvents); Expect.equals(ITERATIONS, _closeEvents); break; case 3: + Expect.equals(ITERATIONS, _dataEvents); + Expect.equals(ITERATIONS, _closeEvents); + break; case 4: + Expect.equals(ITERATIONS, _closeEvents); + break; case 5: case 6: + case 7: + case 8: Expect.equals(ITERATIONS, _dataEvents); Expect.equals(ITERATIONS, _closeEvents); break; @@ -168,13 +197,23 @@ class SocketClose { SendPort _sendPort; Socket _socket; List _buffer; + int _readBytes; int _dataEvents; int _closeEvents; int _errorEvents; int _iterations; int _mode; + int _donePort; } + +class ConnectionData { + ConnectionData(Socket this.connection) : readBytes = 0; + Socket connection; + int readBytes; +} + + class SocketCloseServer extends Isolate { static final HOST = "127.0.0.1"; @@ -183,36 +222,71 @@ class SocketCloseServer extends Isolate { void main() { - void connectionHandler(Socket connection) { + void connectionHandler(ConnectionData data) { + var connection = data.connection; + + void readBytes(whenFiveBytes) { + var read = connection.inputStream.read(); + data.readBytes += read.length; + if (data.readBytes == 5) { + whenFiveBytes(); + } + } void dataHandler() { - _dataEvents++; switch (_mode) { case 0: Expect.fail("No data expected"); break; case 1: - connection.inputStream.read(); + readBytes(() { + _dataEvents++; + }); break; case 2: - connection.inputStream.read(); - connection.inputStream.close(); + readBytes(() { + _dataEvents++; + connection.inputStream.close(); + }); break; case 3: - connection.inputStream.read(); - connection.outputStream.write("Hello".charCodes()); - connection.inputStream.close(); - //connection.outputStream.close(); + readBytes(() { + _dataEvents++; + connection.outputStream.write("Hello".charCodes()); + connection.outputStream.noPendingWriteHandler = () { + connection.inputStream.close(); + }; + }); break; case 4: - connection.inputStream.read(); - connection.outputStream.write("Hello".charCodes()); + readBytes(() { + _dataEvents++; + connection.outputStream.write("Hello".charCodes()); + connection.inputStream.close(); + }); break; case 5: + readBytes(() { + _dataEvents++; + connection.outputStream.write("Hello".charCodes()); + }); + break; case 6: - connection.inputStream.read(); - connection.outputStream.write("Hello".charCodes()); - connection.outputStream.close(); + case 7: + readBytes(() { + _dataEvents++; + connection.outputStream.write("Hello".charCodes()); + connection.outputStream.noPendingWriteHandler = () { + connection.outputStream.close(); + }; + }); + break; + case 8: + readBytes(() { + _dataEvents++; + connection.outputStream.write("Hello".charCodes()); + connection.outputStream.close(); + }); break; default: Expect.fail("Unknown test mode"); @@ -221,7 +295,7 @@ class SocketCloseServer extends Isolate { void closeHandler() { _closeEvents++; - connection.close(); + connection.outputStream.close(); } void errorHandler() { @@ -243,9 +317,11 @@ class SocketCloseServer extends Isolate { // Make sure all iterations have been run. In multiple of these // scenarios it is possible to get the SERVERSHUTDOWN message // before we have received the last close event on the - // server. We therefore always wait for the correct number of + // server. In these cases we wait for the correct number of // close events. - if (_iterations == ITERATIONS && _closeEvents == ITERATIONS) { + if (_iterations == ITERATIONS && + (_closeEvents == ITERATIONS || + (_mode == 2 || _mode == 3 || _mode == 4))) { switch (_mode) { case 0: Expect.equals(0, _dataEvents); @@ -257,12 +333,14 @@ class SocketCloseServer extends Isolate { break; case 2: case 3: - Expect.equals(ITERATIONS, _dataEvents); - Expect.equals(ITERATIONS, _closeEvents); - break; case 4: + Expect.equals(ITERATIONS, _dataEvents); + Expect.equals(0, _closeEvents); + break; case 5: case 6: + case 7: + case 8: Expect.equals(ITERATIONS, _dataEvents); Expect.equals(ITERATIONS, _closeEvents); break; @@ -272,13 +350,16 @@ class SocketCloseServer extends Isolate { Expect.equals(0, _errorEvents); _server.close(); this.port.close(); + _donePort.send(null); } else { new Timer(waitForResult, 100); } } this.port.receive((message, SendPort replyTo) { + _donePort = replyTo; if (message != SERVERSHUTDOWN) { + _readBytes = 0; _errorEvents = 0; _dataEvents = 0; _closeEvents = 0; @@ -286,7 +367,10 @@ class SocketCloseServer extends Isolate { _mode = message; _server = new ServerSocket(HOST, 0, 10); Expect.equals(true, _server !== null); - _server.connectionHandler = connectionHandler; + _server.connectionHandler = (connection) { + var data = new ConnectionData(connection); + connectionHandler(data); + }; _server.errorHandler = errorHandlerServer; replyTo.send(_server.port, null); } else { @@ -296,6 +380,8 @@ class SocketCloseServer extends Isolate { } ServerSocket _server; + SendPort _donePort; + int _readBytes; int _errorEvents; int _dataEvents; int _closeEvents; @@ -310,14 +396,20 @@ main() { // 1: Client sends and closes. // 2: Client sends. Server closes. // 3: Client sends. Server responds and closes. - // 4: Client sends and half-closes. Server responds and closes. - // 5: Client sends. Server responds and half closes. - // 6: Client sends and half-closes. Server responds and half closes. - new SocketClose.start(0); - new SocketClose.start(1); - new SocketClose.start(2); - new SocketClose.start(3); - new SocketClose.start(4); - new SocketClose.start(5); - new SocketClose.start(6); + // 4: Client sends. Server responds and closes without waiting for everything + // being sent. + // 5: Client sends and half-closes. Server responds and closes. + // 6: Client sends. Server responds and half closes. + // 7: Client sends and half-closes. Server responds and half closes. + // 8: Client sends and half-closes. Server responds and half closes without + // explicitly waiting for everything being sent. + var tests = 9; + var port = new ReceivePort(); + var completed = 0; + port.receive((message, ignore) { + if (++completed == tests) port.close(); + }); + for (var i = 0; i < tests; i++) { + new SocketClose.start(i, port.toSendPort()); + } }