[libcxx-commits] [libcxx] [libc++] Fix filebuf resetting its underlying buffer upon close() (PR #168947)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jan 9 08:36:01 PST 2026
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/168947
>From 1fc552d5f4dc059cda87ba9d2ed7b594ed0c0d14 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 20 Nov 2025 16:00:12 -0500
Subject: [PATCH 1/6] [libc++] Fix filebuf resetting its underlying buffer upon
close()
When closing a filebuf, we would previously call setbuf(0, 0). Not
only does this get rid of any underlying buffer set by the user, but
it also sets the unbuffered mode.
This means that if the filebuf is then reopened to keep writing to a
file, it would have lost track of the original user-provided buffer
(or the default one which has a size of 4096), and instead it would
use the unbuffered mode, which is terribly slow.
This led to a bug report where users were complaining that closing and
reopening the filebuf led to a significantly worse performance than
using it without having closed and reopened. While this is a slightly
unusual usage pattern, it should definitely work.
rdar://161833214
---
libcxx/include/fstream | 7 +-
.../close.dont-get-rid-of-buffer.pass.cpp | 132 ++++++++++++++++++
2 files changed, 138 insertions(+), 1 deletion(-)
create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 90e35740c17cf..acce9a026abf1 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -778,7 +778,12 @@ basic_filebuf<_CharT, _Traits>* basic_filebuf<_CharT, _Traits>::close() {
if (fclose(__h.release()))
__rt = nullptr;
__file_ = nullptr;
- setbuf(0, 0);
+ // Reset the get and the put areas without nonetheless getting rid of the
+ // underlying buffers, which might have been configured by the user who might
+ // reopen the stream.
+ this->setg(nullptr, nullptr, nullptr);
+ this->setp(nullptr, nullptr);
+ __cm_ = __no_io_operations;
}
return __rt;
}
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
new file mode 100644
index 0000000000000..6c2dcefdaaa37
--- /dev/null
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
@@ -0,0 +1,132 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// This test requires the fix to std::filebuf::close() (which is defined in the
+// built library) from PR XXX.
+// UNSUPPORTED: using-built-library-before-llvm-22
+
+// <fstream>
+
+// basic_filebuf<charT,traits>* close();
+
+//
+// Ensure that basic_filebuf::close() does not get rid of the underlying buffer set
+// via pubsetbuf(). Otherwise, reopening the stream will result in not reusing the
+// same buffer, which might be conforming but is definitely surprising. The standard
+// is not very clear on whether that is actually conforming.
+//
+
+#include <cassert>
+#include <fstream>
+#include <string>
+
+#include "platform_support.h"
+#include "test_macros.h"
+
+struct overflow_detecting_filebuf : std::filebuf {
+ explicit overflow_detecting_filebuf(bool* overflow_monitor) : did_overflow_(overflow_monitor) {
+ assert(overflow_monitor != nullptr && "must provide an overflow monitor");
+ }
+
+ using Traits = std::filebuf::traits_type;
+ virtual std::filebuf::int_type overflow(std::filebuf::int_type ch = Traits::eof()) {
+ *did_overflow_ = true;
+ return std::filebuf::overflow(ch);
+ }
+
+private:
+ bool* did_overflow_;
+};
+
+int main(int, char**) {
+ {
+ std::string temp = get_temp_file_name();
+
+ bool did_overflow;
+ overflow_detecting_filebuf buf(&did_overflow);
+
+ // Set a custom buffer (of size 32, reused below)
+ char underlying_buffer[32];
+ buf.pubsetbuf(underlying_buffer, sizeof(underlying_buffer));
+
+ // (1) Open a file and insert a first character. That should overflow() and set the underlying
+ // put area to our internal buffer set above.
+ {
+ buf.open(temp, std::ios::out | std::ios::trunc);
+ did_overflow = false;
+ buf.sputc('c');
+ assert(did_overflow == true);
+ }
+
+ // (2) Now, confirm that we can still insert 30 more characters without calling
+ // overflow, since we should be writing to the internal buffer.
+ {
+ did_overflow = false;
+ for (int i = 0; i != 30; ++i) {
+ buf.sputc('c');
+ assert(did_overflow == false);
+ }
+ }
+
+ // (3) Writing the last character may or may not call overflow(), depending on whether
+ // the library implementation wants to flush as soon as the underlying buffer is
+ // full, or on the next attempt to insert. For libc++, it doesn't overflow yet.
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(!did_overflow);
+ }
+
+ // (4) Writing one-too-many characters will overflow (with libc++).
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(did_overflow);
+ }
+
+ // Close the stream. This should NOT unset the underlying buffer we set at the beginning
+ // Unfortunately, the only way to check that is to repeat the above tests which tries to
+ // tie the presence of our custom set buffer to whether overflow() gets called. This is
+ // not entirely portable since implementations are free to call overflow() whenever they
+ // want, but in practice this works pretty portably.
+ buf.close();
+
+ // Repeat (1)
+ {
+ buf.open(temp, std::ios::out | std::ios::trunc);
+ did_overflow = false;
+ buf.sputc('c');
+ assert(did_overflow == true);
+ }
+
+ // Repeat (2)
+ {
+ did_overflow = false;
+ for (int i = 0; i != 30; ++i) {
+ buf.sputc('c');
+ assert(did_overflow == false);
+ }
+ }
+
+ // Repeat (3)
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(!did_overflow);
+ }
+
+ // Repeat (4)
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(did_overflow);
+ }
+ }
+
+ return 0;
+}
>From 0c46e5d203bdd24355cfa1afdd71e9552cf07e9b Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 20 Nov 2025 16:14:44 -0500
Subject: [PATCH 2/6] Add PR link
---
.../filebuf.members/close.dont-get-rid-of-buffer.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
index 6c2dcefdaaa37..9f7c3c5015ea7 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
@@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
// This test requires the fix to std::filebuf::close() (which is defined in the
-// built library) from PR XXX.
+// built library) from https://github.com/llvm/llvm-project/pull/168947.
// UNSUPPORTED: using-built-library-before-llvm-22
// <fstream>
>From 04191a6d717e8695b0f0cf01f5b24ecb47c30cb3 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 5 Jan 2026 09:07:23 -0500
Subject: [PATCH 3/6] Remove extra scope
---
.../close.dont-get-rid-of-buffer.pass.cpp | 136 +++++++++---------
1 file changed, 67 insertions(+), 69 deletions(-)
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
index 9f7c3c5015ea7..3020b4439cecb 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
@@ -44,89 +44,87 @@ struct overflow_detecting_filebuf : std::filebuf {
};
int main(int, char**) {
- {
- std::string temp = get_temp_file_name();
-
- bool did_overflow;
- overflow_detecting_filebuf buf(&did_overflow);
+ std::string temp = get_temp_file_name();
- // Set a custom buffer (of size 32, reused below)
- char underlying_buffer[32];
- buf.pubsetbuf(underlying_buffer, sizeof(underlying_buffer));
+ bool did_overflow;
+ overflow_detecting_filebuf buf(&did_overflow);
- // (1) Open a file and insert a first character. That should overflow() and set the underlying
- // put area to our internal buffer set above.
- {
- buf.open(temp, std::ios::out | std::ios::trunc);
- did_overflow = false;
- buf.sputc('c');
- assert(did_overflow == true);
- }
+ // Set a custom buffer (of size 32, reused below)
+ char underlying_buffer[32];
+ buf.pubsetbuf(underlying_buffer, sizeof(underlying_buffer));
- // (2) Now, confirm that we can still insert 30 more characters without calling
- // overflow, since we should be writing to the internal buffer.
- {
- did_overflow = false;
- for (int i = 0; i != 30; ++i) {
- buf.sputc('c');
- assert(did_overflow == false);
- }
- }
+ // (1) Open a file and insert a first character. That should overflow() and set the underlying
+ // put area to our internal buffer set above.
+ {
+ buf.open(temp, std::ios::out | std::ios::trunc);
+ did_overflow = false;
+ buf.sputc('c');
+ assert(did_overflow == true);
+ }
- // (3) Writing the last character may or may not call overflow(), depending on whether
- // the library implementation wants to flush as soon as the underlying buffer is
- // full, or on the next attempt to insert. For libc++, it doesn't overflow yet.
- {
- did_overflow = false;
+ // (2) Now, confirm that we can still insert 30 more characters without calling
+ // overflow, since we should be writing to the internal buffer.
+ {
+ did_overflow = false;
+ for (int i = 0; i != 30; ++i) {
buf.sputc('c');
- LIBCPP_ASSERT(!did_overflow);
+ assert(did_overflow == false);
}
+ }
- // (4) Writing one-too-many characters will overflow (with libc++).
- {
- did_overflow = false;
- buf.sputc('c');
- LIBCPP_ASSERT(did_overflow);
- }
+ // (3) Writing the last character may or may not call overflow(), depending on whether
+ // the library implementation wants to flush as soon as the underlying buffer is
+ // full, or on the next attempt to insert. For libc++, it doesn't overflow yet.
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(!did_overflow);
+ }
- // Close the stream. This should NOT unset the underlying buffer we set at the beginning
- // Unfortunately, the only way to check that is to repeat the above tests which tries to
- // tie the presence of our custom set buffer to whether overflow() gets called. This is
- // not entirely portable since implementations are free to call overflow() whenever they
- // want, but in practice this works pretty portably.
- buf.close();
-
- // Repeat (1)
- {
- buf.open(temp, std::ios::out | std::ios::trunc);
- did_overflow = false;
- buf.sputc('c');
- assert(did_overflow == true);
- }
+ // (4) Writing one-too-many characters will overflow (with libc++).
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(did_overflow);
+ }
- // Repeat (2)
- {
- did_overflow = false;
- for (int i = 0; i != 30; ++i) {
- buf.sputc('c');
- assert(did_overflow == false);
- }
- }
+ // Close the stream. This should NOT unset the underlying buffer we set at the beginning
+ // Unfortunately, the only way to check that is to repeat the above tests which tries to
+ // tie the presence of our custom set buffer to whether overflow() gets called. This is
+ // not entirely portable since implementations are free to call overflow() whenever they
+ // want, but in practice this works pretty portably.
+ buf.close();
- // Repeat (3)
- {
- did_overflow = false;
- buf.sputc('c');
- LIBCPP_ASSERT(!did_overflow);
- }
+ // Repeat (1)
+ {
+ buf.open(temp, std::ios::out | std::ios::trunc);
+ did_overflow = false;
+ buf.sputc('c');
+ assert(did_overflow == true);
+ }
- // Repeat (4)
- {
- did_overflow = false;
+ // Repeat (2)
+ {
+ did_overflow = false;
+ for (int i = 0; i != 30; ++i) {
buf.sputc('c');
- LIBCPP_ASSERT(did_overflow);
+ assert(did_overflow == false);
}
}
+ // Repeat (3)
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(!did_overflow);
+ }
+
+ // Repeat (4)
+ {
+ did_overflow = false;
+ buf.sputc('c');
+ LIBCPP_ASSERT(did_overflow);
+ }
+
return 0;
}
>From 3079a6733e1d7376fa1c00439bb6e20e558b1052 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 5 Jan 2026 09:16:43 -0500
Subject: [PATCH 4/6] Make the behavior an extension (pedantically)
---
libcxx/docs/ImplementationDefinedBehavior.rst | 7 +++++++
.../filebuf.members/close.dont-get-rid-of-buffer.pass.cpp | 3 +--
2 files changed, 8 insertions(+), 2 deletions(-)
rename libcxx/test/{std => extensions/libcxx}/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp (96%)
diff --git a/libcxx/docs/ImplementationDefinedBehavior.rst b/libcxx/docs/ImplementationDefinedBehavior.rst
index 62f715a354c00..97ea96a506363 100644
--- a/libcxx/docs/ImplementationDefinedBehavior.rst
+++ b/libcxx/docs/ImplementationDefinedBehavior.rst
@@ -62,6 +62,13 @@ E.g.,
- ``std::hermite(unsigned n, T x)`` for ``n >= 128``
+`[filebuf.virtuals] <https://eel.is/c++draft/filebuf.virtual>`_ Effect of calling ``basic_filebuf::setbuf`` with nonzero arguments
+----------------------------------------------------------------------------------------------------------------------------------
+
+Libc++ uses the provided buffer as the underlying buffer for input and output, and
+does not discard that buffer even when the stream is closed.
+
+
`[stringbuf.cons] <http://eel.is/c++draft/stringbuf.cons>`_ Whether sequence pointers are initialized to null pointers
----------------------------------------------------------------------------------------------------------------------
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
similarity index 96%
rename from libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
rename to libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
index 3020b4439cecb..233d466e011ad 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
+++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
@@ -17,8 +17,7 @@
//
// Ensure that basic_filebuf::close() does not get rid of the underlying buffer set
// via pubsetbuf(). Otherwise, reopening the stream will result in not reusing the
-// same buffer, which might be conforming but is definitely surprising. The standard
-// is not very clear on whether that is actually conforming.
+// same buffer, which is conforming but also very surprising.
//
#include <cassert>
>From 6976d2fbdef00b54c4a3217134c03940f8019237 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 5 Jan 2026 09:18:19 -0500
Subject: [PATCH 5/6] Improve comment
---
libcxx/include/fstream | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 66f1257f33112..ec7b633b86be4 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -759,9 +759,9 @@ basic_filebuf<_CharT, _Traits>* basic_filebuf<_CharT, _Traits>::close() {
if (fclose(__h.release()))
__rt = nullptr;
__file_ = nullptr;
- // Reset the get and the put areas without nonetheless getting rid of the
- // underlying buffers, which might have been configured by the user who might
- // reopen the stream.
+ // Reset the get and the put areas without getting rid of the underlying buffers,
+ // which might have been configured by the user. Make sure to keep the buffers
+ // since the user may re-open the stream.
this->setg(nullptr, nullptr, nullptr);
this->setp(nullptr, nullptr);
__cm_ = __no_io_operations;
>From e59356190775c25f7a7acacc0ea46a2067d1d8d6 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 9 Jan 2026 11:30:23 -0500
Subject: [PATCH 6/6] Mark test as unsupported on no-filesystem and
no-localization
---
.../filebuf.members/close.dont-get-rid-of-buffer.pass.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
index 233d466e011ad..495af6cc38506 100644
--- a/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
+++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/close.dont-get-rid-of-buffer.pass.cpp
@@ -10,6 +10,8 @@
// built library) from https://github.com/llvm/llvm-project/pull/168947.
// UNSUPPORTED: using-built-library-before-llvm-22
+// UNSUPPORTED: no-localization, no-filesystem
+
// <fstream>
// basic_filebuf<charT,traits>* close();
More information about the libcxx-commits
mailing list