Bug #18077: Marshal.dump(closed_io) raises IOError instead of TypeError

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.1.0dev (2021-08-16T08:00:19Z master a8714b83c4) [x86_64-linux]
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[larskanis (Lars Kanis)]

Marshal.dump is expected to raise a TypeError for unmarshallable objects. But closed streams raise an IOError:

$ ruby -e "io=IO.pipe.first; io.close; Marshal.dump(io)"
-e:1:in `internal_encoding': closed stream (IOError)
	from -e:1:in `dump'
	from -e:1:in `<main>'

This issue is present in all current ruby versions.

[nobu (Nobuyoshi Nakada)]

One concern for that [PR], probably may not be a matter, is closed IOs will no longer raise IOError on other Encoding operations too.
[PR]: https://github.com/ruby/ruby/pull/4746

$ ./ruby -v -e 'p Encoding.compatible?(Encoding::US_ASCII, File.open(IO::NULL).tap(&:close))'
ruby 3.1.0dev (2021-08-16T08:00:19Z master a8714b83c4) [x86_64-darwin19]
-e:1:in `internal_encoding': closed stream (IOError)
	from -e:1:in `compatible?'
	from -e:1:in `<main>'

$ ./ruby -v -e 'p Encoding.compatible?(Encoding::US_ASCII, File.open(IO::NULL).tap(&:close))'
ruby 3.1.0dev (2021-08-16T12:33:34Z master 9dd58a421b) [x86_64-darwin19]
last_commit=Fix Marshal.dump(closed_io) to raise TypeError
nil

[larskanis (Lars Kanis)]

https://github.com/ruby/ruby/pull/4749 is another fix without the above side effect. Is it OK?

[Eregon (Benoit Daloze)]

Why does IO#internal_encoding (and external_encoding) raise if the IO is closed?
They could just return the encoding, it's still stored in in the IO instance, and we are not trying to access the fd, right?

JRuby 9.2.17.0 does not raise, and I think it is the better behavior here.

[mame (Yusuke Endoh)]

I don't have any strong opinion but @eregon's approach looks a bit better.

[larskanis (Lars Kanis)]

@Eregon (Benoit Daloze) This was my first though as well, but the current behavior is already defined in ruby-spec.

I would prefer the behavioral change of IO#(in|ex)ternal_encoding being usable on closed IOs and prepared a PR: https://github.com/ruby/ruby/pull/4758

[nobu (Nobuyoshi Nakada)]

Since this is a bug report, I think that the old example in rubyspec should be removed.

[larskanis (Lars Kanis)]

I removed the old example from https://github.com/ruby/ruby/pull/4758 .

[Eregon (Benoit Daloze)]

larskanis (Lars Kanis) wrote in #note-5:

@Eregon (Benoit Daloze) This was my first though as well, but the current behavior is already defined in ruby-spec.

We can change it in ruby/spec (adding version guards and adding the new expectation).
And in fact I would be glad if the adapted specs would test the more sensible behavior.
ruby/spec often tests whatever the behavior of CRuby is (for best compatibility with other Rubies), it might not always make sense.

[Eregon (Benoit Daloze)]

I should have read the PR first.
Yes, removing the old example and adding the new one with a version guard is perfect here :)
That allows other implementations & older versions to potentially use the new behavior already, and there is probably very little to no code relying on the previous exception.

• Status changed from Open to Closed

Applied in changeset git|6594623f623a0982da62cc105094da0701d499da.

---

Fix Marshal.dump(closed_io) to raise TypeError and allow encoding on closed IO

Mashalling a closed IO object raised "closed stream (IOError)" before instead of TypeError.
This changes IO#(in|ex)ternal_encoding to still return the encoding even if the underlying FD is closed.

Fixes bug #18077