Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for .NET Core 3.1? #114

Closed
meinsiedler opened this issue Mar 9, 2021 · 9 comments
Closed

Support for .NET Core 3.1? #114

meinsiedler opened this issue Mar 9, 2021 · 9 comments
Assignees
Milestone

Comments

@meinsiedler
Copy link
Contributor

Hi,

we are having issues after upgrading from .NET 4.7 to .NET Core 3.1. With .NET Core 3.1 it seems like nothing gets written to the COM port. We don't receive any errors or exceptions but no data shows up.

The issue is easily reproducible with the following sample code:

static void Main(string[] args)
{
	Console.WriteLine("Start");

	using (var serialPortStream = new SerialPortStream("COM1", 9600, 8, Parity.None, StopBits.One))
	{
		serialPortStream.Open();

		Console.WriteLine("Serial Port Opened");

		Console.WriteLine("Wait for write. Enter any key.");
		Console.ReadKey();


		var bytes = new byte[] { 0x01, 0x02, 0x03 };

		serialPortStream.WriteAsync(bytes, 0, bytes.Length);
		serialPortStream.FlushAsync();

		Console.WriteLine("Finished.");
	}
}

When using <TargetFramework>net472</TargetFramework> in the .csproj file, everything works as expected and the bytes get written to the COM port. When switching to <TargetFramework>netcoreapp3.1</TargetFramework> no data gets written to the COM port.

We are using a serial port monitoring tool to check if any data gets written. In the .NET Core 3.1 case, the data doesn't show up in our monitoring tool.

We use Windows 10 and the latest SerialPortStream version 2.2.2.

Unfortunately, we can't provide any further details, since we don't receive any errors and the only change is the TargetFramework. If there's anything more we can provide, please let us know.

@jcurl
Copy link
Owner

jcurl commented Mar 9, 2021

Hi, do you have a test program that also includes the solution file? Either for VSCode or Visual Studio?

I suspect the problem is seen because SerialPortStream doesn't have native implementations for ReadAsync or WriteAsync, so the base Stream class needs to do this. If you look at the code, Read/Write is for all frameworks, BeginRead/BeginWrite is only for .NET Framework (and the framework has wrappers for ReadAsync/WriteAsync). .NET Core doesn't provide BeginRead/BeginWrite and there's no ReadAsync/WriteAsync which means that .NET Core isn't compatible with the current implementation.

Best would be to put a breakpoint in the SerialPortStream.Write method and see if it's being called.

@meinsiedler
Copy link
Contributor Author

Hi,

thanks for your response. While trying to reproduce the issue again, I noticed that I forgot to await the two calls in the sample code above.

await serialPortStream.WriteAsync(bytes, 0, bytes.Length);
await serialPortStream.FlushAsync();

When awaiting the two calls, everything works as expected with netcoreapp3.1 too.

I will close the issue for now, because it seems that the issue lies somewhere else in our code base and isn't reproducible with a simple sample program.

Sorry for the inconvenience and thanks again.

@jcurl
Copy link
Owner

jcurl commented Mar 9, 2021

No problem, glad that in principle it should work.

@meinsiedler
Copy link
Contributor Author

Hi,

we further investigated our issue and now we could really reproduce it with a simple sample program.

We have a complex program where we set up the reading and writing from the COM port in different threads so that we can provide bi-directional communication. With .NET 4.7 it was possible to await the ReadAsync in one thread and at the same time call the WriteAsync in another thread. This doesn't work any longer with .NET Core 3.1. The await WriteAsync call blocks and never finishes. It seems that the await ReadAsync call from the other thread waits for something to read and blocks the writing on the other thread.

static async Task Main(string[] args)
{
	Console.WriteLine("Start");

	using (var serialPortStream = new SerialPortStream("COM1", 9600, 8, Parity.None, StopBits.One))
	{
		serialPortStream.Open();

		Console.WriteLine("Serial Port Opened");

		var buffer = new byte[1024];
		var readTask = Task.Run(async () => await serialPortStream.ReadAsync(buffer, 0, buffer.Length));

		Console.WriteLine("Wait for write. Enter any key.");
		Console.ReadKey();
		

		await Task.Run(async () =>
		{
			var bytes = new byte[] { 0x01, 0x02, 0x03 };

			Console.WriteLine("Write started");
			await serialPortStream.WriteAsync(bytes, 0, bytes.Length);
			await serialPortStream.FlushAsync();
			Console.WriteLine("Write finished");
		});

		Console.WriteLine("Awaiting read...");

		await readTask;
		

		Console.WriteLine("Finished.");
	}
}

I've also attached the solution. When changing the <TargetFramework> in the .csproj file to net472 everything works as expected and we can write to the stream while waiting for reading from the stream.

SerialPortStreamTest.zip

@meinsiedler meinsiedler reopened this Mar 10, 2021
@jcurl
Copy link
Owner

jcurl commented Mar 11, 2021

Hi, this is really starting to sound like something I wrote about 8 years ago. See https://www.codeproject.com/Tips/575618/Avoiding-Deadlocks-with-System-IO-Stream-BeginRead

I know the framework doesn't like read/write on the same thread. This is why I implemented the Begin|EndRead/Begin|EndWrite on .NET Desktop. But .NET Core doesn't have this and I suspect the framework is deadlocking itself.

So it looks like the only solution for .NET Core is to provide an implementation of the AsyncRead/Write. You could confirm this by providing a backtrace at the time of the deadlock. Secondly, if you have a small test program (I'm sorry - I'm very busy with my day job, and when I get time, I could use a test case that is confirmed as failing to ensure a solution).

@meinsiedler
Copy link
Contributor Author

meinsiedler commented Mar 11, 2021

Hi,
thank you for your effort! This is indeed an interesting article and sheds some light on the issue.

I could use a test case that is confirmed as failing to ensure a solution

In case you have overlooked it, I have attached a solution with a simple test in form of an executable console app in my previous comment. This should quickly show the difference between netcoreapp3.1 and net472. If it helps and is more practical, I'll to provide a unit test project with failing tests.

We are using your package professionally (not in a side/pet project) and are very interested in fixing the issue. We are more than happy to help wherever possible.

@meinsiedler
Copy link
Contributor Author

Hi,

I created a repository with a unit test project which shows the issue: https://github.com/meinsiedler/SerialPortStreamTest

Please also take a look at the README where I describe my findings.

Please reach out to us if we can help.

@jcurl
Copy link
Owner

jcurl commented Mar 12, 2021

Hello, thanks for providing the repository. I've forked it,. Please understand that my professional situation requires more than usual engagement at the same time I'm spending on my non-work hours on training, so I don't know when I can get to this, probably not in the next few weeks.. If it is as I suspect, an implementation of ReadAsync/WriteAsync will need to be done, and as I use the .NET 4.0 solution, whatever is written will need to be compatible with that.

It shouldn't be too hard, the main I/O loop sets semaphores in the buffer object, it would be figuring out how to use that and synchronize with a Task object. As you've indicated that this is a professional package, you might want to consider forking and providing a solution yourself, or hiring someone to provide this solution depending on your needs. Looking at the code for BeginRead/BeginWrite would be a good place to start. The CancellationToken stuff might be harder.

I'll leave this ticket open, at least to highlight there is now a real-world usecase that requires Async (there's another ticket, but with no impact mentioned).

@jcurl jcurl self-assigned this Apr 2, 2021
@jcurl jcurl added this to the v2.3.0 milestone Apr 2, 2021
@jcurl jcurl linked a pull request Apr 2, 2021 that will close this issue
@jcurl jcurl added the in-work label Apr 2, 2021
jcurl added a commit that referenced this issue Apr 13, 2021
jcurl pushed a commit that referenced this issue Apr 13, 2021
@jcurl jcurl removed the in-work label Apr 13, 2021
@jcurl
Copy link
Owner

jcurl commented Apr 13, 2021

Integrated with bc94184.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants