Skip to content

Fix Posix Serial Driver Not Unblocking Read on Close #1166

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

Merged
merged 1 commit into from
Mar 31, 2024
Merged

Fix Posix Serial Driver Not Unblocking Read on Close #1166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions openc3/lib/openc3/io/posix_serial_driver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
# GNU Affero General Public License for more details.

# Modified by OpenC3, Inc.
# All changes Copyright 2022, OpenC3, Inc.
# All changes Copyright 2024, OpenC3, Inc.
# All Rights Reserved
#
# This file may also be used under the terms of a commercial license
# This file may also be used under the terms of a commercial license
# if purchased from OpenC3, Inc.

require 'fcntl'
Expand Down Expand Up @@ -82,12 +82,17 @@ def initialize(port_name = '/dev/ttyS0',
tio.ospeed = baud_rate
@handle.tcflush(Termios::TCIOFLUSH)
@handle.tcsetattr(Termios::TCSANOW, tio)

@pipe_reader, @pipe_writer = IO.pipe
@readers = [@handle, @pipe_reader]
end

# (see SerialDriver#close)
def close
if @handle
# Close the serial Port
@pipe_writer.write('.')
@pipe_writer.close
@handle.close
@handle = nil
end
Expand Down Expand Up @@ -132,9 +137,19 @@ def read
begin
data = @handle.read_nonblock(65535)
rescue Errno::EAGAIN, Errno::EWOULDBLOCK
result = IO.fast_select([@handle], nil, nil, @read_timeout)
if result
retry
begin
read_ready, _ = IO.fast_select(@readers, nil, nil, @read_timeout)
rescue IOError
@pipe_reader.close unless @pipe_reader.closed?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how closing the pipe_reader is going to help with the serial port handle

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing the pipe reader is just to make sure we don't leave file handles open. Return "" means that we are dead and should no longer be used going forward. IOError should only occur if the serial port has already been closed.

return ""
end
if read_ready
if read_ready.include?(@pipe_reader)
@pipe_reader.close unless @pipe_reader.closed?
return ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you close the pipe_reader if the read_ready says it's ready?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pipe_reader is only ever ready, if pipe_writer was written. That means we have been closed.

else
retry
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This jumps back to the read_nonblock right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

end
else
raise Timeout::Error, "Read Timeout"
end
Expand Down
Loading