Bug #53439 EndOfStreamException is not thrown when net_write_timeout exceeded
Submitted: 5 May 2010 15:32 Modified: 13 Jul 2010 14:47
Reporter: Pavel Bazanov Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / NET Severity:S3 (Non-critical)
Version:6.3.2 (actually revision 760+) OS:Any
Assigned to: Vladislav Vaintroub CPU Architecture:Any
Tags: EndOfStreamException, net_write_timeout

[5 May 2010 15:32] Pavel Bazanov
Description:
Hello, 
This bug is closely related to bug45978.
After bug45978 was fixed, the mysql connector would throw EndOfStreamException when net_write_timeout exceeded. But recently I decided to add tests for all bugs that I reported and I found out that expected EndOfStreamException is not thrown anymore, instead only few rows are returned by DataReader and then DataReader.Read() just returns false like there are not more rows (though the query should return more rows). 

You can see the test case in How to repeat section.

The actual queue is:

1) net_write_timeout exceeds
2) MySqlStream.ReadFully() throws EndOfStreamException
3) MySqlStream.LoadPacket() catches the exception, wraps it into MySqlException and throws it
4) ResultSet.NextRow() catches the exception and swallows it:

public bool NextRow(CommandBehavior behavior)
{
  ...
  try
  {
    fetched = GetNextRow();
  }
  catch (MySqlException ex)
  {
    // so we get here
    if (ex.Number == 1317) fetched = false;
  }

  if (!fetched)
  {
    // and then we get here
    // and we just return false like reading is completed and there is simply no more rows to read
    readDone = true;
    return false;
  }
  ...
}

5) MySqlDataReader.Read() simply returns false without notifying about exception.

I believe this is a bug, because client code doesn't even know error/exception is occurred. So the client code thinks everything is ok, but it's not. The exception should be thrown up through call stack so the client code will be able to catch it and decide what to do.

I checked the source code and it seems the bug was commited in revision 760. See the suggested fix.

How to repeat:
[Test, ExpectedException(typeof(MySqlException))]
public void Bug45978ShouldNotAppearAnymore()
{
	using (MySqlConnection conn = DB.ConnectToDb())
	{
		var command = new MySqlCommand("", conn);
		command.CommandText = "SET net_write_timeout = 1";
		command.ExecuteNonQuery();
		// let's read from the big table with 1M rows
		command.CommandText = "SELECT * FROM parts LIMIT 100000";
		using (MySqlDataReader reader = command.ExecuteReader())
		{
			Thread.Sleep(10000);
			int rowsRead = 0;
			object lastReadFieldValue; // just for debugging purposes
			while (reader.Read())
			{
				lastReadFieldValue = reader["PartN"];
				rowsRead++;
			}
			Debug.WriteLine(rowsRead); // prints 142, instead of 100000
		}
	}
}

Suggested fix:
Revision 760 changed this code in ResultSet.NextRow():

if (!driver.FetchDataRow(reader.Statement.StatementId, 0, Size))
{
  readDone = true;
  return false;
}

to this new code:

bool fetched = false;
try
{
  fetched = driver.FetchDataRow(reader.Statement.StatementId, 0, Size);
}
catch (MySqlException ex)
{
  if (ex.Number == 1317) fetched = false;
}

if (!fetched)
{
  readDone = true;
  return false;
}

So, as you can see, the previous code didn't swallow exceptions and the new code does.

I believe the code should look like this:

public bool NextRow(CommandBehavior behavior)
{
	...
	bool fetched = false;
	try
	{
			fetched = GetNextRow();
	}
	catch (MySqlException ex)
	{
		if (ex.Number == 1317) 
			fetched = false;
		else
    			throw;
	}

	if (!fetched)
	{
			readDone = true;
			return false;
	}
	...
}
[6 May 2010 11:22] Tonci Grgin
Hello Pavel.

Some people like to see the exception thrown, some not but I think you are right here. Let me confirm your findings.
[6 May 2010 11:23] Tonci Grgin
Note, Bug#53457
[6 May 2010 11:32] Tonci Grgin
Verified just as described with test case provided against latest sources.
[7 Jun 2010 11:57] marnik marnik
I'm using 6.1.3.0 and experience the same bug.
[29 Jun 2010 20:55] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/112499

866 Vladislav Vaintroub	2010-06-29
      Ensure exceptions are not silently eaten inside MySqlDataReader.Read()  (Bug#53439)
[29 Jun 2010 21:07] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/112500

866 Vladislav Vaintroub	2010-06-29
      Ensure exceptions are not silently eaten inside MySqlDataReader.Read()  (Bug#53439)
[1 Jul 2010 22:40] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/112709

822 Vladislav Vaintroub	2010-07-02
      Ensure exceptions are not silently eaten inside MySqlDataReader.Read()  (Bug#53439)
[1 Jul 2010 22:41] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/112710

822 Vladislav Vaintroub	2010-07-02
      Ensure exceptions are not silently eaten inside MySqlDataReader.Read()  (Bug#53439)
[1 Jul 2010 22:56] Vladislav Vaintroub
pushed to 6.1, 6.2, 6.3
[8 Jul 2010 21:03] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/113179

871 Vladislav Vaintroub	2010-07-08 [merge]
      Ensure exceptions are not silently eaten inside MySqlDataReader.Read()  (Bug#53439)
[13 Jul 2010 14:47] Tony Bedford
An entry has been added to the 6.1.5, 6.2.4, and 6.3.3 changelogs:

MySQL Connector/NET did not throw an EndOfStreamException exception when net_write_timeout was exceeded.