Commit bed98921 authored by honghaiz@chromium.org's avatar honghaiz@chromium.org

Another attempt to fix a Chrome crash.

From the bug report and the other relevant bug 236392 reported by ASan,
The problem arises when ReadData() had some errors or completed, and then freed
the NetworkStats, but later the callback of the Write() is started on the freed
object. This most likely happened when the initial ReadData() had some errors.
So I did two modifications.
1. For the first time when ReadData is called, I checked the return value.
Only if it is ERR_IO_PENDING, it continues to Send requests.
2. The Write() callback is on a WeakPtr. If the NetworkStats object is freed,
the callback will be a no-op.

BUG=273917

Review URL: https://chromiumcodereview.appspot.com/23875004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221644 0039d316-1c4b-4281-b951-d872f2087c98
parent 7aa0d381
...@@ -219,8 +219,9 @@ bool NetworkStats::ConnectComplete(int result) { ...@@ -219,8 +219,9 @@ bool NetworkStats::ConnectComplete(int result) {
} }
if (start_test_after_connect_) { if (start_test_after_connect_) {
ReadData(); // This ReadData() reads data for all HelloReply and all // ReadData() reads data for all HelloReply and all subsequent probe tests.
// subsequent probe tests. if (ReadData() != net::ERR_IO_PENDING)
return false;
SendHelloRequest(); SendHelloRequest();
} else { } else {
// For unittesting. Only run the callback, do not destroy it. // For unittesting. Only run the callback, do not destroy it.
...@@ -291,9 +292,9 @@ void NetworkStats::SendProbeRequest() { ...@@ -291,9 +292,9 @@ void NetworkStats::SendProbeRequest() {
TestPhaseComplete(WRITE_FAILED, result); TestPhaseComplete(WRITE_FAILED, result);
} }
void NetworkStats::ReadData() { int NetworkStats::ReadData() {
if (!socket_.get()) if (!socket_.get())
return; return 0;
int rv = 0; int rv = 0;
do { do {
...@@ -305,6 +306,7 @@ void NetworkStats::ReadData() { ...@@ -305,6 +306,7 @@ void NetworkStats::ReadData() {
kMaxMessageSize, kMaxMessageSize,
base::Bind(&NetworkStats::OnReadComplete, base::Unretained(this))); base::Bind(&NetworkStats::OnReadComplete, base::Unretained(this)));
} while (rv > 0 && !ReadComplete(rv)); } while (rv > 0 && !ReadComplete(rv));
return rv;
} }
void NetworkStats::OnReadComplete(int result) { void NetworkStats::OnReadComplete(int result) {
...@@ -315,7 +317,8 @@ void NetworkStats::OnReadComplete(int result) { ...@@ -315,7 +317,8 @@ void NetworkStats::OnReadComplete(int result) {
// loop. // loop.
base::MessageLoop::current()->PostDelayedTask( base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, FROM_HERE,
base::Bind(&NetworkStats::ReadData, weak_factory_.GetWeakPtr()), base::Bind(base::IgnoreResult(&NetworkStats::ReadData),
weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(1)); base::TimeDelta::FromMilliseconds(1));
} }
} }
...@@ -420,7 +423,7 @@ int NetworkStats::SendData(const std::string& output) { ...@@ -420,7 +423,7 @@ int NetworkStats::SendData(const std::string& output) {
int bytes_written = socket_->Write( int bytes_written = socket_->Write(
write_buffer_.get(), write_buffer_.get(),
write_buffer_->BytesRemaining(), write_buffer_->BytesRemaining(),
base::Bind(&NetworkStats::OnWriteComplete, base::Unretained(this))); base::Bind(&NetworkStats::OnWriteComplete, weak_factory_.GetWeakPtr()));
if (bytes_written < 0) if (bytes_written < 0)
return bytes_written; return bytes_written;
UpdateSendBuffer(bytes_written); UpdateSendBuffer(bytes_written);
......
...@@ -142,8 +142,9 @@ class NetworkStats { ...@@ -142,8 +142,9 @@ class NetworkStats {
void OnWriteComplete(int result); void OnWriteComplete(int result);
// Read data from server until an error or IO blocking occurs or reading is // Read data from server until an error or IO blocking occurs or reading is
// complete. // complete. Return the result value from socket reading and 0 if |socket_|
void ReadData(); // is Null.
int ReadData();
// Send data contained in |str| to server. // Send data contained in |str| to server.
// Return a negative value if IO blocking occurs or there is an error. // Return a negative value if IO blocking occurs or there is an error.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment