[libc++] Avoid calling setlocale in do_unshift when unnecessary

[m417z]

This is an attempt to mitigate #110954.

As part of the libc++.dll initialization, static DoIOSInit is initialized:

llvm-project/libcxx/src/iostream.cpp

Lines 165 to 167 in 5bdee35

	ios_base::Init::Init() {
	static DoIOSInit init_the_streams; // gets initialized once
	}

When the dll is unloaded or on process shutdown, DoIOSInit::~DoIOSInit is called. It ends up calling flush:

llvm-project/libcxx/src/iostream.cpp

Line 159 in 5bdee35

wcout_ptr->flush();

Which calls pubsync:

llvm-project/libcxx/include/__ostream/basic_ostream.h

Line 659 in 5bdee35

if (this->rdbuf()->pubsync() == -1)

Which ends up calling do_unshift:

llvm-project/libcxx/src/locale.cpp

Lines 1475 to 1479 in 5bdee35

	codecvt<wchar_t, char, mbstate_t>::result codecvt<wchar_t, char, mbstate_t>::do_unshift(
	state_type& st, extern_type* to, extern_type* to_end, extern_type*& to_nxt) const {
	to_nxt = to;
	extern_type tmp[MB_LEN_MAX];
	size_t n = __locale::__wcrtomb(tmp, intern_type(), &st, __l_);

Which, as can be seen, unconditionally calls __locale::__wcrtomb, which ends up calling setlocale via __libcpp_locale_guard.

All this means that setlocale is called on process shutdown even if wcout is never used, or even if nothing stream-related is used. Calling setlocale on process shutdown causes problems, as described in the mentioned issue.

This PR is an attempt to avoid calling setlocale in the vast majority of cases, when there's no output to be flushed. It's not a complete fix to the issue, but it will make it much less common, and it will at least allow to flush output manually to avoid the issue if streams are used.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Michael Maltsev (m417z)

Changes

This is an attempt to mitigate #110954.

As part of the libc++.dll initialization, static DoIOSInit is initialized:

llvm-project/libcxx/src/iostream.cpp

Lines 165 to 167 in 5bdee35

	ios_base::Init::Init() {
	static DoIOSInit init_the_streams; // gets initialized once
	}

When the dll is unloaded or on process shutdown, DoIOSInit::~DoIOSInit is called. It ends up calling flush:

llvm-project/libcxx/src/iostream.cpp

Line 159 in 5bdee35

wcout_ptr->flush();

Which calls pubsync:

llvm-project/libcxx/include/__ostream/basic_ostream.h

Line 659 in 5bdee35

if (this->rdbuf()->pubsync() == -1)

Which ends up calling do_unshift:

llvm-project/libcxx/src/locale.cpp

Lines 1475 to 1479 in 5bdee35

	codecvt<wchar_t, char, mbstate_t>::result codecvt<wchar_t, char, mbstate_t>::do_unshift(
	state_type& st, extern_type* to, extern_type* to_end, extern_type*& to_nxt) const {
	to_nxt = to;
	extern_type tmp[MB_LEN_MAX];
	size_t n = __locale::__wcrtomb(tmp, intern_type(), &st, __l_);

Which, as can be seen, unconditionally calls __locale::__wcrtomb, which ends up calling setlocale via __libcpp_locale_guard.

All this means that setlocale is called on process shutdown even if wcout is never used, or even if nothing stream-related is used. Calling setlocale on process shutdown causes problems, as described in the mentioned issue.

This PR is an attempt to avoid calling setlocale in the vast majority of cases, when there's no output to be flushed. It's not a complete fix to the issue, but it will make it much less common, and it will at least allow to flush output manually to avoid the issue if streams are used.

---

Full diff: https://github.com/llvm/llvm-project/pull/117153.diff

1 Files Affected:

• (modified) libcxx/src/locale.cpp (+2)

diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index a1e10401f0b299..5ecd99c53cd516 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -1475,6 +1475,8 @@ codecvt<wchar_t, char, mbstate_t>::result codecvt<wchar_t, char, mbstate_t>::do_
 codecvt<wchar_t, char, mbstate_t>::result codecvt<wchar_t, char, mbstate_t>::do_unshift(
     state_type& st, extern_type* to, extern_type* to_end, extern_type*& to_nxt) const {
   to_nxt = to;
+  if (std::mbsinit(&st))
+    return ok;
   extern_type tmp[MB_LEN_MAX];
   size_t n = __locale::__wcrtomb(tmp, intern_type(), &st, __l_);
   if (n == size_t(-1) || n == 0) // on error

[cpplearner]

I wonder if we should remove the unshift call in sync().

llvm-project/libcxx/include/fstream

Lines 965 to 983 in c4d656a

	template <class _CharT, class _Traits>
	int basic_filebuf<_CharT, _Traits>::sync() {
	if (__file_ == nullptr)
	return 0;
	if (!__cv_)
	__throw_bad_cast();
	if (__cm_ & ios_base::out) {
	if (this->pptr() != this->pbase())
	if (overflow() == traits_type::eof())
	return -1;
	codecvt_base::result __r;
	do {
	char* __extbe;
	__r = __cv_->unshift(__st_, __extbuf_, __extbuf_ + __ebs_, __extbe);
	size_t __nmemb = static_cast<size_t>(__extbe - __extbuf_);
	if (fwrite(__extbuf_, 1, __nmemb, __file_) != __nmemb)
	return -1;
	} while (__r == codecvt_base::partial);

The standard doesn't require such a call. [filebuf.virtuals]/19:

int sync() override;

Effects: If a put area exists, calls filebuf​::​overflow to write the characters to the file, then flushes the file as if by calling fflush(file). If a get area exists, the effect is implementation-defined.