Disposed XamlXmlReader does not close the internal file reader for some constructors

[michaelw-au]

Description

After constructing with XamlXmlReader(string, XamlXmlReaderSettings), reading with it and disposing it...
If you try to open the same file for write you get an IOException "The process cannot access the file ... because it is being used by another process."

Same issue for the XamlXmlReader(Stream, XamlXmlReaderSettings) form.

Even when you specify CloseInput=true on the XamlXmlReaderSettings

Reproduction Steps

Precondition: You need a Test.xaml file for it to read from

var testFile = "Test.xaml";
var settings = new XamlXmlReaderSettings() 
{
    // Actually want other settings here like ProvideLineInfo = true
    CloseInput = true 
};

using (var reader = new XamlXmlReader(testFile, settings))
{
    // Read something to ensure it has opened things
    var firstRead = reader.Read();

}
// XamlXmlReader was Disposed. It should be safe to open the same file for write.
using var readStream = File.OpenWrite(testFile);

Attaching a sample Test.xaml.txt file (because it won't let me upload a .xaml file) - rename it to Test.xaml to use it.
Test.xaml.txt

Expected behavior

With these constructor forms, once the XamlXmlReader is Disposed, user should be able to open the same file for write.

Actual behavior

With these constructor forms, once the XamlXmlReader is Disposed, an exception occurs when the user tries to open the same file for write.

Regression?

No response

Known Workarounds

Instead user does

• creates a Stream themselves,
• pass that to XamlXmlReader constructor (ideally with XamlXmlReaderSettings.CloseInput = false in case this issue gets fixed)
• Dispose of XamlXmlReader
• Dispose of the Stream themselves.

Configuration

.NET 6
(specific cases tested on net6.0-windows, x64, running on Windows 10 but not sure any of these are relevant)

Other information

I think property XamlXmlReaderSettings.CloseInput doesn't apply to the first form (taking a string filePath), as nothing else can possibly close whatever the reader used internally to read - so it should always close on Dispose.
Although a different class, it also seems analagous to XmlReaderSettings.CloseInput, which says the same.

The property should apply to the second form (taking a Stream argument), but seems to be ineffective there too.

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

After constructing with XamlXmlReader(string, XamlXmlReaderSettings), reading with it and disposing it...
If you try to open the same file for write you get an IOException "The process cannot access the file ... because it is being used by another process."

Same issue for the XamlXmlReader(Stream, XamlXmlReaderSettings) form.

Even when you specify CloseInput=true on the XamlXmlReaderSettings

Reproduction Steps

Precondition: You need a Test.xaml file for it to read from

var testFile = "Test.xaml";
var settings = new XamlXmlReaderSettings() 
{
    // Actually want other settings here like ProvideLineInfo = true
    CloseInput = true 
};

using (var reader = new XamlXmlReader(testFile, settings))
{
    // Read something to ensure it has opened things
    var firstRead = reader.Read();

}
// XamlXmlReader was Disposed. It should be safe to open the same file for write.
using var readStream = File.OpenWrite(testFile);

Attaching a sample Test.xaml.txt file (because it won't let me upload a .xaml file) - rename it to Test.xaml to use it.
Test.xaml.txt

Expected behavior

With these constructor forms, once the XamlXmlReader is Disposed, user should be able to open the same file for write.

Actual behavior

With these constructor forms, once the XamlXmlReader is Disposed, an exception occurs when the user tries to open the same file for write.

Regression?

No response

Known Workarounds

Instead user does

• creates a Stream themselves,
• pass that to XamlXmlReader constructor (ideally with XamlXmlReaderSettings.CloseInput = false in case this issue gets fixed)
• Dispose of XamlXmlReader
• Dispose of the Stream themselves.

Configuration

.NET 6
(specific cases tested on net6.0-windows, x64, running on Windows 10 but not sure any of these are relevant)

Other information

I think property XamlXmlReaderSettings.CloseInput doesn't apply to the first form (taking a string filePath), as nothing else can possibly close whatever the reader used internally to read - so it should always close on Dispose.
Although a different class, it also seems analagous to XmlReaderSettings.CloseInput, which says the same.

The property should apply to the second form (taking a Stream argument), but seems to be ineffective there too.

Author:	michaelw-au
Assignees:	-
Labels:	area-System.IO, untriaged
Milestone:	-

[miloush]

In all cases the setting is transferred to the underlying XmlReader via XmlReaderSettings.CloseInput. Unfortunately XamlXmlReader does not close/dispose the XmlReader for the setting to apply, which it probably should, similarly how BAML reader also deriving from XamlReader does.

This has compatibility implications though, because by default (if no settings are supplied) the close input is set to true, so this would suddenly be closing readers/streams that used to stay open.

[michaelw-au]

That's a good point about changing behavior. Then it seems fair to leave it with current behavior for the constructor that accepts a Stream.

I still think it should close / dispose the underlying XmlReader when you use the constructor that takes a string (file path) argument. In that case nothing else has access to the internal stream/XmlReader, so nothing else can do so.
Meaning that constructor will cause issues if you want to modify the file after disposing the XamlXmlReader (or e.g. rename/move it). It's not obvious to the user that this is the cause.