@vonzshik since you're working with timeouts, you cant take a look at this. Otherwise, I could take this a bit later since already did some SSL stuff.

@YohDeadfall sure I can take a look - but doesn't it mean, that ReadBuffer has the same problem?

Yes, it has. The buffer is just a wrapper around the stream.

Interestingly enough, it seems to work fine for .net 5, but not for .net 4.6.1

Oh, very good! If it also works for .NET 3.1 we could just close this issue because .NET Framework is on the way to a graveyard.

You mean that SslStream actually works well with timeouts now? BTW I assume we're discussing sync timeouts, I wonder if async ones (via cancellation tokens) work?

Related (but the outcome of that issue isn't clear): dotnet/runtime#22496

Oh, very good! If it also works for .NET 3.1 we could just close this issue because .NET Framework is on the way to a graveyard.

I think we can just conditional-compile the check to only throw for TFMs where the timeout doesn't work (i.e. net461).

I would also assume that it doesn't work on .NET Core 3.1 because of dotnet/runtime#22496

I would also assume that it doesn't work on .NET Core 3.1 because of dotnet/runtime#22496

It actually does work - results.

[Test]
public void WaitWithSsl()
{
	var csb = new NpgsqlConnectionStringBuilder(ConnectionString)
	{
		SslMode = SslMode.Require,
		TrustServerCertificate = true,
	};
	using (var conn = OpenConnection(csb.ToString()))
	{
		conn.ExecuteNonQuery("LISTEN notifytest");
		Assert.That(conn.Wait(1000), Is.EqualTo(false));
		Assert.That(conn.ExecuteScalar("SELECT 1"), Is.EqualTo(1));
	}
}

Well then... great! So I guess just conditionally-compile the check for net461?

(Technically there's also 2.x, with 2.1 being still supported. We can check what's going on there if we want to be complete)

You mean that SslStream actually works well with timeouts now? BTW I assume we're discussing sync timeouts, I wonder if async ones (via cancellation tokens) work?

Nope for async ones, as .net 4.6.1 doesn't support the cancellation on the socket level through the cancellation token.

Right, but maybe SslStream does support cancellation tokens in .NET 5.0, just like they improved NetworkStream to do it?

I'm not sure how it would help. The problem is, SslStream (and maybe NetworkStream, have not checked yet) uses default implementation from Stream for async methods. And this implementation flatly ignores the cancellation token you supply (.net 4.8).

NetworkStream definitely does honor cancellation tokens (see dotnet/corefx#36516), otherwise this whole work we're doing would have been for nothing :)

SslStream also seems to flow cancellation tokens, and so maybe honors them too (see ReadAsync, and then this. We can test this to be sure.

@roji I'm talking about .net 4.6.1, not .net core. AFAIK, it still uses the same libraries from .net 4.6.1.

And that is why every test with the cancellation token is disabled for .net 4.6.1 - they would be stuck forever.

Also, wouldn't it in case of ,net standard 2.0 and 2.1 be entirely dependent on runtime?

@roji I'm talking about .net 4.6.1, not .net core. AFAIK, it still uses the same libraries from .net 4.6.1.

Ah OK, I misunderstood.

And that is why every test with the cancellation token is disabled for .net 4.6.1 - they would be stuck forever.

For sync Wait, we've been throwing up to now if a timeout is specified but the connection is secure. I was proposing that if we keep that behavior but only for .NET 4.6.1, the test could test it (i.e. that we throw in those conditions). In other words, instead of not have the test at all for NET461, we could have a version that asserts that we throw the right exception. If we do this, I think it makes sense to also do the same for async.

Also, wouldn't it in case of ,net standard 2.0 and 2.1 be entirely dependent on runtime?

Good point - we indeed shouldn't go into differentiating .NET Framework from .NET Core at runtime.

For sync Wait, we've been throwing up to now if a timeout is specified but the connection is secure. I was proposing that if we keep that behavior but only for .NET 4.6.1, the test could test it (i.e. that we throw in those conditions). In other words, instead of not have the test at all for NET461, we could have a version that asserts that we throw the right exception. If we do this, I think it makes sense to also do the same for async.

And I've also misunderstood 😄.
The only problem I see is .net standard 2.0, which can be used together with .net 4.6.1 and .net core 2.0. I suppose, there is nothing really we can do.

I agree. We can just let it do the timeout on netstandard2.0 (i.e. not block it), and if the user is on .NET Framework it's their problem...

This sounds like a problem that will slowly fade away on its own. We also haven't gotten many new issues around this, closing.