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.

Sign up for GitHub

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

Jump to bottom

Problem with RD-Server Port-Redirection (ERROR_HANDLE_EOF) #142

Closed
kunostoeckli opened this issue Feb 7, 2024 · 13 comments
Closed

Problem with RD-Server Port-Redirection (ERROR_HANDLE_EOF) #142

kunostoeckli opened this issue Feb 7, 2024 · 13 comments

Comments

@kunostoeckli
Copy link

Hi Jason,

Thanks for the RJCP.DLL.SerialPortStream.

We searched a library for Windows Serial-Ports, which uses Kernel32-File-Read-Functionality.

On some Windows 2022 RD-Server, after a ReadExisting(), the connection got closed.

(Technically: in RJCP.IO.Ports.Native.Windows.CommOverlappedIo, the OverlappedIoThread() got an error and then m_IsRunning = false;
)

I think we found a solution:
after Kernel32.GetOverlappedResult(m_ComPortHandle, ref readOverlapped, out bytes, true);
we get the error 38 (WinError.ERROR_HANDLE_EOF) on the RD-Server.

We think, that this EOF is a legal answer. We tried it in the Version 2.x branch as following:
if ((win32Error != WinError.ERROR_OPERATION_ABORTED && win32Error != WinError.ERROR_HANDLE_EOF) || bytes > 0) {

What do you think: Is the additional ( .. && win32Error != WinError.ERROR_HANDLE_EOF) correct?
If yes: Can you add this to your repository?

Thanks ans best regards!

Kuno.

image
@jcurl
Copy link
Owner

jcurl commented Feb 7, 2024

Looking a bit at the code, it might be related to the character 0x1A arriving over the serial port. As I've never been able to reproduce this with the drivers that I have, could I ask you please to:
a) Let me know the serial port. Is this something I can perhaps buy from Amazon to test?
b) With your proposed changes, are you able to confirm all data arriving? Or would it drop the byte 0x1A?
c) Do you get an event SerialData.Eof from SerialPortStream.DataReceived?

The code change could be made, but then I wonder if an event would be missed, so this will need proper testing.

Regarding an update, I'm in the process of upgrading SerialPortStream 3.x to .NET Core 6.0 (dropping .NET Standard 2.1), so I'll get that work done first, then I hope to test for this change.

@kunostoeckli
Copy link
Author

kunostoeckli commented Feb 8, 2024 via email

@jcurl
Copy link
Owner

jcurl commented Feb 8, 2024

Thanks for the answer. I'll plan to do the update in both 2.x and 3.x

To check out the 3.x code, you must:

  • git checkout RJCP.DLL.SerialPortStream serialportstream
  • git checkout RJCP.DLL.BufferIO bufferio
  • git checkout RJCP.DLL.Trace trace
  • git checkout RJCP.DLL.DeviceMgr devicemgr

Change the RJCP.DLL to include your preferred URL (https, ssh, or whatever). Ensure they're all alongside each other. Then in the serialportstream folder, a dotnet build should be sufficient, and the https://github.com/jcurl/RJCP.DLL.SerialPortStream/blob/master/code/SerialPortStream.csproj finds the relative paths and builds. Everything on HEAD master is fine.

I might have to look into a workaround for the driver giving EOF, but not raising the event. I would have expected the event to be raised.

@kunostoeckli
Copy link
Author

kunostoeckli commented Feb 9, 2024 via email

@jcurl jcurl changed the title https://github.com/jcurl/SerialPortStream: Problem with RD-Server Port-Redirection Problem with RD-Server Port-Redirection Feb 21, 2024
@jcurl jcurl changed the title Problem with RD-Server Port-Redirection Problem with RD-Server Port-Redirection (ERROR_HANDLE_EOF) Feb 21, 2024
jcurl added a commit that referenced this issue Feb 21, 2024
@jcurl
SerialPortStream: Don't close the port on ERROR_HANDLE_EOF
8031438 
It is observed that WinError.ERROR_HANDLE_EOF can occur in normal circumstances and should not result in the serial port stream closing down.

Issue: #142
Issue: DOTNET-975
@jcurl
Copy link
Owner

jcurl commented Feb 21, 2024

Hello, I would appreciate if you would test the branch bugfix/dotnet-975 (based on v2.x). I've provided a fix and refactored slightly to keep code readable.

I will do manual testing on the weekend (I don't have a way to reproduce this issue), and will check for any regressions I might find. All my unit tests pass.

Then after this is tested, I'll backport the changes on to the 3.x branch and provide NuGet releases for all.

@kunostoeckli
Copy link
Author

kunostoeckli commented Feb 23, 2024 via email

@jcurl
Copy link
Owner

jcurl commented Mar 4, 2024

Hi, I appreciate your busy. I'm nearly ready for a new release of all my packages, pending if you have feedback.

@kunostoeckli
Copy link
Author

Hi,
Sorry for the delay.
Ok. We tested the branch bugfix/dotnet-975.

It looks good: without the "case WinError.ERROR_HANDLE_EOF:", we get the error again.
With the branch, it works fine.

Thank you very much for the fix!

Kuno.

@kunostoeckli
Copy link
Author

Hi,
Do you have https://www.buymeacoffee.com/ or something similar.
Best regards
Kuno.

@jcurl
Copy link
Owner

jcurl commented Mar 4, 2024

Thank you! I appreciate your gesture. One of my biggest hopes in Open Source are contributes from people to make the software better. Thank you for not only reporting the problem, but suggesting the solution.

jcurl added a commit that referenced this issue Mar 4, 2024
@jcurl
SerialPortStream: Don't close the port on ERROR_HANDLE_EOF
956636d 
It is observed that WinError.ERROR_HANDLE_EOF can occur in normal circumstances and should not result in the serial port stream closing down.

Issue: #142
Issue: DOTNET-975
@jcurl
Copy link
Owner

jcurl commented Mar 4, 2024

Now released with version 2.4.2 and uploaded in NuGet.

@jcurl jcurl closed this as completed Mar 4, 2024
jcurl added a commit that referenced this issue Mar 4, 2024
@jcurl
SerialPortStream: Don't close the port on ERROR_HANDLE_EOF
ca463f7 
It is observed that WinError.ERROR_HANDLE_EOF can occur in normal circumstances and should not result in the serial port stream closing down.

Issue: #142
Issue: DOTNET-975
@kunostoeckli
Copy link
Author

Thanks a lot!

@kunostoeckli
Copy link
Author

kunostoeckli commented Mar 7, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jcurl @kunostoeckli