-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
SerialPort Flush[Async] makes Read[Async] timeout/block #35545
Comments
Tagging subscribers to this area: @jozkee |
@maloo there used to be one issue with serialstream.flush which I suspect this is a dup of #29597 I have fixed it with dotnet/corefx#40896 |
Yes, could be this issue. I decompiled the assembly deployed and it had the discard instead of drain, see below. I assume my data comes back before flush is called and discard delete the incoming data too, not just the write buffer. I will try using latest. Can I use nuget prerelease of just IO.Ports or I have to use full 5.0 runtime? I just get an exception when just using the latest IO.Ports from 5.0-preview3, I'm running 3.1.201 on the Pi.
public override void Flush()
{
if (this._handle == null)
InternalResources.FileNotOpen();
Interop.Termios.TermiosDiscard(this._handle, Interop.Termios.Queue.AllQueues);
} |
The publish contains the normal System.IO.Ports.Native.so, no libSystem.IO.Ports.Native. |
@maloo actually I forgot this is an OOB library (so no need to use preview) you can probably use https://www.nuget.org/packages/System.IO.Ports/5.0.0-preview.3.20214.6 |
That's what I tried above, but I got the exception. Seems publish and loader don't agree on the name of the native lib. |
@maloo if you're building on Windows for RPI you should |
csproj is above. I tried both framework dependant and independant. When docompiling IO.Ports I can see 5.0 has added the lib prefix in the interop attribute. But the file published still has the old name. I looked in the nuget package but could not find the native so in either version. Is the native so part of the framework? Or where does it come from if not the nuget package? |
I found the native dll in a package called runtime.native.System.IO.Ports. But this package is not set as a dependency of System.IO.Ports, so I get the old native so instead of the new one with a changed name. This will probably give a lot of people some grief if not fixed ;) I guess you create an issue for that? |
If you want to try the app posted above, you need to install the ssh-deploy dotnet tool from https://github.com/unosquare/sshdeploy. Or remove the deploy config from the csproj above. My target dotnet on the Pi is 3.1.201 if that helps. On windows it is 3.1.300-preview-015115, but since a selfcontained publish didn't change it I guess issue is not in different versions in Windows/Linux. |
You should not be touching any dlls directly. Try below csproj instead (note the version of the package and also i'd recommend not putting SelfContained or RID in there and pass it from the command line). I don't know what the sshdeploy does but you should use <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<!--<RuntimeIdentifier>linux-arm</RuntimeIdentifier>-->
<!--<SelfContained>false</SelfContained>-->
</PropertyGroup>
<PropertyGroup>
<SshDeployHost>192.168.1.118</SshDeployHost>
<SshDeployClean />
<SshDeployTargetPath>/home/pi/serialflush</SshDeployTargetPath>
<SshDeployUsername>pi</SshDeployUsername>
<SshDeployPassword>raspberry</SshDeployPassword>
<RunPostBuildEvent>OnBuildSuccess</RunPostBuildEvent>
</PropertyGroup>
<Target Condition="$(BuildingInsideSshDeploy) ==''" Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="cd $(ProjectDir)" />
<Exec Command="dotnet-sshdeploy push" />
</Target>
<ItemGroup>
<PackageReference Include="System.IO.Ports" Version="5.0.0-preview.3.20214.6" />
</ItemGroup>
</Project> runtime.native.System.IO.Ports should have reference to runtime specific package which contains your asset. For question about backporting - all bug fixes between 3.0 and 5.0 can be considered breaking changes - this package is OOB which means there is no reason why any porting should be required. |
This is what I tested and it doesn't publish the correct native dll, becuase of the dependency issue in IO.Ports (using 3.1 version instead of 5.0). linux-arm is the same as command line publish -r linux-arm. ssh-deploy is just a post build step to delete old files on target and scp the new files in publish directory so you can just hit build in VS and have it published directly to Pi. And I have tried selfcontained true and false. The following csproj is what allows me to get correct native so (5.0 instead of 3.1). But then WriteAsync never return, which is very odd ... nothing should be blocking a write to serial port, unless you have changed the defaults of HW handshake, which is a bug in that case and not supported by USB serial ports if I recall correctly. What is the easiest way to debug framework dlls on Pi? Is there any easy steps to setup remote debugging available for Pi/Linux via ssh? <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<RuntimeIdentifier>linux-arm</RuntimeIdentifier>
<!--<SelfContained>false</SelfContained>-->
</PropertyGroup>
<PropertyGroup>
<SshDeployHost>192.168.1.118</SshDeployHost>
<SshDeployClean />
<SshDeployTargetPath>/home/pi/serialflush</SshDeployTargetPath>
<SshDeployUsername>pi</SshDeployUsername>
<SshDeployPassword>raspberry</SshDeployPassword>
<RunPostBuildEvent>OnBuildSuccess</RunPostBuildEvent>
</PropertyGroup>
<Target Condition="$(BuildingInsideSshDeploy) ==''" Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="cd $(ProjectDir)" />
<Exec Command="dotnet-sshdeploy push" />
</Target>
<ItemGroup>
<PackageReference Include="runtime.native.System.IO.Ports" Version="5.0.0-preview.3.20214.6" />
<PackageReference Include="System.IO.Ports" Version="5.0.0-preview.3.20214.6" />
<!--<PackageReference Include="System.IO.Ports" Version="4.7.0" />-->
</ItemGroup>
</Project> |
@maloo this scenario should just work without making any changes to 3.1. For debuggin see: https://www.hanselman.com/blog/RemoteDebuggingWithVSCodeOnWindowsToARaspberryPiUsingNETCoreOnARM.aspx I'd have to try this scenario myself to see what's going but I'm currently during the move and don't have any Raspberry PIs lying around.. @wfurt do you perhaps still have any RPis setup for Serial Ports (or presumably just PC would hit the same issue?) to see if 5.0 preview Serial Ports package works with 3.1? |
Agreed :) Just meant if there are new issues, like WriteAsync blocking. Then maybe porting the Flush fix only to 3.1 track (4.7) could be an option. Since 3.1 is LTS. But first lets see if we can get write working again with the latest nuget... Issue should be reproduceable on Unix/Linux assuming this is the same problem with drain/discard. |
I will try 5.0 later, so we know if it is the 3.1+5.0 that is the issue |
Tested net50 instead of netcoreapp3.1, exactly the same code and csproj except for that change. And now WriteAsync/FlushAsync/ReadAsync all work ... so there must be some issue with this library when used with netcoreapp3.1. But I'm not able to use net50 prerelease in our product, IO.Ports prerelease is already pushing it. |
System.IO.Ports shouldn't depend on anything significant which would make it not work. What error are you getting when using it with netcoreapp3.1? Are the symptomps the same? This sound to me like your app is loading wrong DLL which suggest something wrong with deployment. Could you compare if you're getting same dlls when target 5.0 vs 3.1? I can't decide what will be backported but I can send a request for consideration and might be easier to get approval because this is an OOB package although honestly we should get this to work first and once this works then there is no reason to backport I guess... |
Both native and managed IO.Ports are binary equal in net50 and netcoreapp3.1 publish folders. Using net50 it works. Using netcoreapp3.1 the app above never return from await WriteAsync(). I added another WriteLine to verify that it is WriteAsync that get stuck mixing 3.1 and 5.0. And ssh-deploy delete all files on target before pushing from publish folder so should not be any old files left. And I have never had any issues with ssh-deploy pushing wrong files. Only issue is msbuild/VS2019 not removing files in publish dir when switching between SelfContained true/false. But I've tested both by manually deleting the bin dir before build+deploy of 3.1+5.0. How is msbuild told to pull in the runtime.native nuget? If it is not by dependency from managed IO.Ports, then how? Somehow netcoreapp3.1 seems to pull in the wrong native nuget if I don't specifically reference the native package as above. |
@maloo this is done through dotnet restore (which calls msbuild with /t:Restore which is defined somewhere in the Sdk). System.IO.Ports have dependency on runtime.native.System.IO.Ports (you can see it i.e. https://www.nuget.org/packages/System.IO.Ports). runtime.native.System.IO.Ports represents the package which will take care of getting the native asset but doesn't have the asset itself. If you look at the dependencies here https://www.nuget.org/packages/runtime.native.System.IO.Ports/ you will see it depends on 4 runtime-specific packages which means the dependency is true only if publishing/restoring for the runtime where the dependency is defined - if unsupported runtime is used this library will not work. |
@joperezr seems we have renamed the native assets when we switched to 5.0 which has somehow broke compat between 5.0 OOB packages trying to run on 3.1. Do you perhaps know anything about it or know who might help? |
I did a thorough clean of target and PC project and rebuild+deployed, switching between netcoreapp3.1 and net50 and 100% reproducable, WriteAsync blocks. But listing of ports and Open works. At least no exception at Open and IsOpen is true. No bytes are written to the port, because I can see a light flashing if anything is sent on the USB to RS232 I use right now. I also tried calling Write on SerialPort directly. And I tried using sync Write instead of await WriteAsync. All block. |
@maloo since you got everything setup, could you try using 3.0 package on 5.0 and see if you get missing library exception? (I don't care about the original bug) |
I can see the dependency in your link, I was looking at the latest nuget release. In 5.0 the dependency is missing ... I can try 3.0 IO.Ports in net50. Is that IO.Ports 4.6? I know 4.7 is 3.1. |
Running IO.Ports 4.6 in net50 starts and has the same flush issue as reported for 4.7 which is expected I guess. |
not sure what do you mean, You should be using 4.7 or 5.0-preview package. The version only tells in what timeframe it was shipped. Target frameworks tell you where it should work and both 3.1 and 5.0 are compatible with .NETStandard 2.0. |
the more I look at your problem it seems to me you somehow ended up with System.IO.Ports from 3.1 timeframe and runtime.native.System.IO.Ports from 5.0 timeframe which have different native library names |
You said 3.0 (.Net Core I assume), I've never used that so I don't know which version of IO.Ports matches that. So I assumed it was 4.6 since 4.7 is in .Net Core 3.1. |
No, in that case I will get a loader exception about the name of the library. I have the lib/net50 version of native if I include the native package manually, since the dependency is missing in the net50 nuget. And I have the net50 of managed, since I got the loader error when I had the wrong native, and then no loader error when I got the right native. |
I've talked with @joperezr offline and he has an idea of what's broken (seems we're missing dependencies to the native assets). He'll try to send a patch for preview4 although it might be too late for that already in which case it would be preview5 |
I can use a nightly to test, or if you have it on another nuget source. What issue did @joperezr think he had a solution for?
|
@maloo package issue causing no native assets and errors (5.0 preview package should work on 3.1). We will unlikely take a patch for the old package since 5.0 preview can be used instead. I suspect blocking Write might be either separate issue or also related to Flush |
Ok, but note that it works using net50, both flush and write. And wrong native assets can be worked around by adding native nuget in csproj. So let me know when you think you have a fix in nightly for blocking write in 3.1 using IO.Ports 5.0 and I will test it. I wonder if it could be a timing issue since the exact same SerialPort code works in net50. |
Let's wait for @joperezr's investigation. Either way this is some packaging issue. |
Yes, but since I can fix the packaging issue by adding the dependency (5.0 native) it will not help me at all. So I just wanted to make sure you look at the blocking Write issue too. <PackageReference Include="runtime.native.System.IO.Ports" Version="5.0.0-preview.3.20214.6" />
<PackageReference Include="System.IO.Ports" Version="5.0.0-preview.3.20214.6" /> |
Haven't followed the full thread for this, but there does seem to be a packaging issue here that we probably got since we first moved to .NET 5.0 where we are not emitting a package reference for the runtime.native.System.IO.Ports package any longer (cc: @ericstj) I'll take a look at why that is happening and try to root cause the issue. |
@maloo do you have more details on the blocking issue, I thought you mentioned 5.0 preview fix has fixed it? |
Not much more detail than the same IO.Ports (5.0 managed and native) works fine on net50 but Write/BaseStream.Write/BaseStream.WriteAsync all block in netcoreapp3.1. Same binaries. That's why I don't think this blocking write is a packaging issue (should of course still fix the missing dependency), since the same package works running on net50 ... |
5.0 preview of IO.Ports fixed the Flush issue, so my app works on net50. But I can't release my product on net5.0. So I need IO.Ports 5.0 to work on netcoreapp3.1, but currently it probably fixed the flush issue, but introduced the blocking Write issue. |
So if I understand correctly:
Can you try if regular Write works for you for now so that you unblock yourself - we should start separate issue to figure out what that is since this one is polluted with packaging problems P.S. @joperezr has fixes IO.Ports package issue here: #35591 - I'll see if we can ship the fix with preview 4 |
As I noted before, All form of write to serial port block for 5.0 Ports + 3.1. SerialPort.Write, BaseStream.Write, BaseStream.WriteAsync. |
@maloo ok, good news is that 5.0 package works correctly so I suspect 5.0+3.1 combination should work too (there is no reason it wouldn't - code is the same). It would be good if you've tried debugging this scenario once we got official package and see where it's stuck at |
I didn't have any luck with VSCode debugging remotely, but I managed to get VS2019 to attach remotely to Pi dotnet process. But debugger is a bit flaky, when I do step into it just runs until the blocking spot, which seems to be main waiting for the async task. If I switch to BaseStream.Write(msg) I get a bit further, but when Write calls WriteAsync it happens again, debugger step into doesn't work. Then I tried BaseStream.Wait(msg, 0, msg.Length), here I can debug until it starts the IOLoop. But my breakpoint in IOLoop is never hit. And Write goes into t.GetAwaiter().GetResult(). Isn't this an anti pattern? Is it really ok to call blocking GetResult() here? Can we be sure Task.Factory.StartNew can start a new thread in this case? |
It seems if (_ioLoop == null) is true even when _ioLoop has a value. Open started the first IOLoop. Write is now starting a second one, while the first one is still executing. Not sure this is the real issue, but it doesn't seem right that _ioLoop==null is true when _ioLoop isn't null. |
Seems like IOLoop tries to call Poll. Poll needs to load a native lib, and these have been renamed. So 5.0 IO.Ports tries to load libNative.so instead of Native.so. But since we are running 3.1 it is missing, since these native libs are part of net50. Will this require a dependency on all net50 native libs from IO.Ports? Is it possible to deploy using both new and old versions of native libs? |
So this will explain the problem, but I will let you guys figure out the right solution. Renaming the native libs seems like a bad idea ;) |
This sounds like might be a culprit. Presumably IOLoop tries to read/write, exception occurs, thread exits doesn't allowing any reads/writes to finish and new loop starts when Write is called again... I have an idea how to fix this but need to talk with some people first (I know how to fix it right now but the reason we did it in the first place is for performance reasons so will need to figure out how to do it without penalty on other libraries) |
fair enough. I'm closing this issue since this issue covers 3 things:
|
A call to
SerialPort.BaseStream.Flush[Async]
seems to mess up the state in the serial port base stream so a read following the flush will timeout/block. Write will send data out on the port without a call to Flush, but other classes wrapping streams in .Net Core, likePipeWriter.Create
, will always flush, making this issue a blocker. Code below will work if the call to flush is removed. I used a USB-RS232 adapter on a RaspPi3 with pin 2-3 connected (loopback). I have verified data on oscilloscope with and without flush.@krwq you seem to have worked a lot on the SerialPort for Linux, is this a known issue with Flush breaking the stream?
The text was updated successfully, but these errors were encountered: