0___________
0___________

Reputation: 67835

using directive does not dispose correctly

public async Task<string> sendJsonToPipeAsync(string jsonLRequest)
{
    string jsonLResponse = string.Empty;

    try
    {
        string pipeName = gulHardwareMonitorService.getPipeName();
        using (var pipeClient = new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous))
        {
            await pipeClient.ConnectAsync();

            using (var writer = new StreamWriter(pipeClient, Encoding.UTF8) { AutoFlush = true })
            using (var reader = new StreamReader(pipeClient, Encoding.UTF8))
            {
                // Convert to JSON-L if needed
                string jsonLFormattedRequest = ConvertToJsonLFormat(jsonLRequest);

                await writer.WriteLineAsync(jsonLFormattedRequest);
                logger.log("JSON Request sent to pipe.");

                jsonLResponse = await reader.ReadLineAsync();
                logger.log("Response received from pipe.");

                // Ensure the stream is flushed before closing
                writer.Dispose() ;
                reader.Dispose() ;
            }
        }
    }
    catch (IOException ex)
    {
        logger.log($"I/O Exception in PipeServer: {ex.Message}");
        jsonLResponse = PipeServer.GenerateErrorResponse(ex.Message);
    }
    catch (Exception ex)
    {
        logger.log($"Exception in PipeServer: {ex.Message}");
        jsonLResponse = PipeServer.GenerateErrorResponse(ex.Message);
    }

    return jsonLResponse;
}

If I do not dispose of manually reader and writer I get an exception that I access closed pipe. As far as I understand they should be disposed at the end of 'using`. Is this disposal delayed or maybe it is a bug in the .NET.

Upvotes: 0

Views: 104

Answers (2)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186813

It seems that the problem is in the order. We should have writer be disposed first (it wants to Flush and must have non-disposed stream) and only then reader and pipeClient (they should check if stream is already disposed and will do nothing).

When you put explicit Dispose(), the order is correct:

writer.Dispose(); // writer is disposed first (it can do its Flush)
reader.Dispose();

When you let using call Dispose for you, the order is wrong:

using (var writer = new StreamWriter(pipeClient, Encoding.UTF8) { AutoFlush = true })
using (var reader = new StreamReader(pipeClient, Encoding.UTF8)) 
{
  ...

  // reader is disposed first 
  // writer wants to Flush but fails (stream is disposed and we have the exception)
}

I suggest reordering:

// writer is created last and disposed first 
using (var reader = new StreamReader(pipeClient, Encoding.UTF8))
using (var writer = new StreamWriter(pipeClient, Encoding.UTF8) { AutoFlush = true })
{
   ...
}

Edit:

Another possibility is to let pipeClient but neither reader nor writer dispose the stream:

// Neither writer, nor reader dispose the stream
// Since the stream is not disposed until pipeClient will do it
// writer can do its Flush()
using (var writer = new StreamWriter(
  pipeClient, Encoding.UTF8, leaveOpen : true) { AutoFlush = true })
using (var reader = new StreamReader(pipeClient, Encoding.UTF8, leaveOpen : true)) 
{
   ...
}

Upvotes: 4

Guga Rukhadze
Guga Rukhadze

Reputation: 60

Main case of using "using" block statement is for the unmanaged resources to be disposed automatically when their scope is ended. So calling dispose explicitly and using the using block is not really necessary at the same time.

If I were you I would remove these explicit dispose statements and debug the code from there. Code should look like this:

using (var writer = new StreamWriter(pipeClient, Encoding.UTF8, 1024, true) { AutoFlush = true })
using (var reader = new StreamReader(pipeClient, Encoding.UTF8, true, 1024, true))
{
    await writer.WriteLineAsync(jsonLFormattedRequest);
    await pipeClient.FlushAsync();
    logger.log("JSON Request sent to pipe.");

    jsonLResponse = await reader.ReadLineAsync();
    logger.log("Response received from pipe.");
}

Upvotes: 1

Related Questions