fstream reuse on Windows causes a null pointer access violation

[KazNX]

Re-using nowide::fstream after a close() and open() can attempt null pointer access while writing to the second file.

A full project has been attached with instructions in the readme. Below is an excert of the code which will reproduce the issue.

void testNowideBug(bool move_assign_stream)
{
  const std::string test_data = /* [snip] 795 character long string literal */;

  nowide::ofstream test_stream_out("out1.txt", std::ios_base::binary);
  test_stream_out.write(test_data.data(), test_data.size());
  test_stream_out.close();
  // Writing to a new file using the same stream will crash.
  // It's fine if you change the test to "hello world".
  // It's fine if you use a new stream object
  if (move_assign_stream)
  {
    test_stream_out = nowide::ofstream("out2.txt", std::ios_base::binary);
  }
  else
  {
    test_stream_out.open("out2.txt", std::ios_base::binary);
  }
  test_stream_out.write(test_data.data(), test_data.size());
}

The final line will fail, the program will crash and out2.txt will be empty. The test string in the example has been removed for breivity, but I've used two strings of 795 characters (I doubt it's that specified) with the same failed results. The string "hellow world" works as expected.

This appears to be specific to Windows and does not affect Ubuntu 18.04 or 20.04.

Testing using MSC 16.11.11 (Visual Studio 2019).

nowide.zip

[KazNX]

For refrence, the issue presents itself in basic_filebuf::overflow() : https://github.com/boostorg/nowide/blob/standalone/include/nowide/filebuf.hpp#L240

The buffer_ pointer is null.

[KazNX]

Also, the length is a furphie. Different lengths produce crashes in other locations. It also seems that "hello world" passing was an artefact from my debugging and is not true.

[Flamefire]

Thanks for the issue report.
This is caused by a bug in the close function which has been fixed a while ago with b96294d
However an unrelated bug caused the standalone branch to not be automatically updated which has been fixed by a944d22 and fccb720 is the update of the standalone branch containing the fix.

I tested the provided code against the current standalone branch via the following commands on Windows inside the extracted folder (didn't want to install the Python and colcon stack):

git clone --branch standalone https://github.com/boostorg/nowide.git
cmake -S nowide -B nw-build -DCMAKE_CXX_STANDARD=17 -DCMAKE_INSTALL_PREFIX=nw-install
cmake --build nw-build --config Debug --target install
cmake -S nowidebug -B nw-build2 -DCMAKE_CXX_STANDARD=17 -DCMAKE_INSTALL_PREFIX=nw-install
cmake --build nw-build2 --config Debug
nw-build2/Debug/nowidebug.exe

Which yields:

Testing with move assignment
...ok
Testing with stream re-use (close/open)
...ok

Anyway I'll include a test case based on your code to avoid this reappearing. If you want to make sure you are using a (standalone) version where this is fixed, require a version of 11.1.1 in the CMake find_package call which is the release that includes the fix.

[KazNX]

Thanks for the quick response.

Could you please retest using a Release build? All the testing I did yesterday was using the tip of the standalone branch - my CMakeLists.txt file for nowide shows it's version 11.1.4 - and it was failing on a release build. I've tried a Debug build as you did above and yes, that does indeed pass.

So it looks like we have a release mode only bug.

[Flamefire]

I retested the above workflow with a release build, i.e. replacing --config Debug by --config Release and it passed.

I also manually verified the code concluding that it is logically impossible for buffer_ to be null at that point in the current develop & standalone branch. The relevant part of the fix are those 2 lines: b96294d#diff-5abbcdd10563140cb03ad1a9d61a29bc562974225d8bc6888c931c59719d4676R177-R178

So I suspect that you somehow ended up using an older version in your test. Check the actually used include paths for the build using nowide that it uses the expected path and that it includes the fix, i.e. those 2 lines (link to current tip of the standalone branch:

nowide/include/nowide/filebuf.hpp

Lines 177 to 178 in 960c201

	setg(0, 0, 0);
	setp(0, 0);

)

So could you please check this? As mentioned I added a test case to the tests based on your code and it passes too, see #151

[KazNX]

Thanks for retesting.

I've checked my includes and run a clean build and my release build is passing. I can't definitely account for the discrepancy, but I suspect I may have had some residual paths in my powershell environment when I ran my first tests.

Thanks for the help.