Skip to content

Commit

Permalink
Fix scn::input deadlock
Browse files Browse the repository at this point in the history
  • Loading branch information
eliaskosunen committed Nov 27, 2023
1 parent 81ca2c4 commit 5f316d3
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 23 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/arch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ jobs:

exclude:
# FIXME: these runs take too long, and time out
- arch: aarch64
- arch: ppc64le
#- arch: aarch64
#- arch: ppc64le

steps:
- name: Checkout
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ jobs:
- cxx: g++-12
os: 20.04
# FIXME?: simdutf has problems with clang 6 and 7 (_ktestc_mask64_u8 undefined)
- cxx: clang++-7
- cxx: clang++-6.0
#- cxx: clang++-7
#- cxx: clang++-6.0
# FIXME: Locally unreproducible ICE
- cxx: clang++-9
#- cxx: clang++-9
# FIXME: some weird linker issue: undefined reference to range_default_scanner default constructor
- cxx: g++-11
std: 20
os: 20.04
#- cxx: g++-11
# std: 20
# os: 20.04
# FIXME?: weird incompatibility with libstdc++ 13 inside <chrono>
# see https://github.com/actions/runner-images/issues/8659
- cxx: clang++-14
Expand Down
6 changes: 5 additions & 1 deletion docs/pages/mainpage.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,13 @@ scnlib internally depends on
<a href="https://github.com/fastfloat/fast_float">fast_float</a> and
<a href="https://github.com/simdutf/simdutf">simdutf</a>.

The CMake machinery automatically fetches, builds, and links these libraries through `FetchContent`.
By default, the CMake machinery automatically fetches, builds, and links these libraries through `FetchContent`.
These libraries are only used in the implementation, and they are not visible to the users of the library.

Alternatively, by setting the CMake options `SCN_USE_EXTERNAL_FAST_FLOAT` or `SCN_USE_EXTERNAL_SIMDUTF` to `ON`,
these libraries are searched for using `find_package`. Use these options, if you already have these libraries
installed.

If your standard library doesn't have an available C++20 `<ranges>` implementation,
a single-header version of <a href="https://github.com/tcbrindle/nanorange">NanoRange</a>
is also bundled with the library, inside the directory `include/scn/external`.
Expand Down
2 changes: 1 addition & 1 deletion include/scn/detail/input_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace scn {

static stdin_subrange impl(const stdin_view& v, priority_tag<4>)
{
SCN_EXPECT(v.is_this_locked());
SCN_EXPECT(v.owns_lock());
return {v};
}

Expand Down
22 changes: 11 additions & 11 deletions include/scn/detail/stdin_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,45 +217,45 @@ namespace scn {

~stdin_view()
{
if (is_this_locked()) {
if (owns_lock()) {
m_manager->auto_sync();
release();
unlock();
}
}

void acquire()
void lock()
{
if (m_manager->m_require_locking) {
SCN_EXPECT(!is_this_locked());
SCN_EXPECT(!owns_lock());
m_lock.lock();
}
}
SCN_NODISCARD bool try_acquire()
SCN_NODISCARD bool try_lock()
{
if (m_manager->m_require_locking) {
SCN_EXPECT(!is_this_locked());
SCN_EXPECT(!owns_lock());
return m_lock.try_lock();
}
return true;
}
SCN_NODISCARD bool is_this_locked() const
SCN_NODISCARD bool owns_lock() const
{
if (m_manager->m_require_locking) {
return m_lock.owns_lock();
}
return true;
}
void release()
void unlock()
{
if (m_manager->m_require_locking) {
SCN_EXPECT(is_this_locked());
m_lock.release();
SCN_EXPECT(owns_lock());
m_lock.unlock();
}
}

stdin_manager& manager()
{
SCN_EXPECT(is_this_locked());
SCN_EXPECT(owns_lock());
return *m_manager;
}

Expand Down
5 changes: 3 additions & 2 deletions src/scn/vscan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ namespace scn {
auto reader = impl::integer_reader<char>{0, base};
SCN_TRY(_, reader.read_source(ranges::subrange(beg, source.end()),
std::is_signed_v<T>));
SCN_UNUSED(_);
SCN_TRY(n, reader.parse_value(value));
return ranges::next(beg, n);
}
Expand All @@ -406,7 +407,7 @@ namespace scn {
-> vscan_result<detail::stdin_subrange>
{
auto source = detail::stdin_manager_instance().make_view();
source.acquire();
source.lock();
auto it = vscan_internal(detail::stdin_subrange{source}, format, args);
if (SCN_UNLIKELY(!it)) {
return unexpected(it.error());
Expand All @@ -423,7 +424,7 @@ namespace scn {
-> vscan_result<detail::stdin_subrange>
{
auto source = detail::stdin_manager_instance().make_view();
source.acquire();
source.lock();
auto it = vscan_internal(detail::stdin_subrange{source}, format, args,
detail::locale_ref{loc});
if (SCN_UNLIKELY(!it)) {
Expand Down
20 changes: 20 additions & 0 deletions tests/unittests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,23 @@ add_executable(scn_impl_tests
target_link_libraries(scn_impl_tests PRIVATE
scn_gtest scn_tests_base scn_internal)
add_test(NAME scn_impl_tests COMMAND scn_impl_tests)

add_executable(scn_stdin_test
main.cpp

stdin_test.cpp
)
target_link_libraries(scn_stdin_test scn_gtest scn_tests_base)

add_custom_target(scn_stdin_test_prepare ALL
COMMAND ${CMAKE_COMMAND} -E copy
"${CMAKE_CURRENT_LIST_DIR}/stdin_test_runner.sh"
"${CMAKE_CURRENT_LIST_DIR}/stdin_test_input.txt"
"${CMAKE_BINARY_DIR}/tests/unittests"
COMMENT "Copying stdin test runner"
)

find_program(BASH_PROGRAM bash)
if (BASH_PROGRAM)
add_test(NAME scn_stdin_test COMMAND ${BASH_PROGRAM} "${CMAKE_BINARY_DIR}/tests/unittests/stdin_test_runner.sh")
endif()
82 changes: 82 additions & 0 deletions tests/unittests/stdin_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright 2017 Elias Kosunen
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// This file is a part of scnlib:
// https://github.com/eliaskosunen/scnlib

#include <scn/scan.h>
#include <iostream>

#include "wrapped_gtest.h"

template <typename T>
static std::optional<T> read_scn()
{
auto r = scn::input<T>("{}");
if (!r) {
return std::nullopt;
}
return r->value();
}
template <typename T>
static std::optional<T> read_scanf()
{
if constexpr (std::is_same_v<T, int>) {
int i{};
if (std::scanf("%d", &i) != 1) {
return std::nullopt;
}
return i;
}
else {
std::string val{};
val.resize(3);
if (std::scanf(" %3c", &val[0]) != 1) {
return std::nullopt;
}
return val;
}
}
template <typename T>
static std::optional<T> read_cin()
{
T i{};
if (!(std::cin >> i)) {
return std::nullopt;
}
return i;
}

using testing::Optional;

TEST(Stdin, Test)
{
using namespace std::string_literals;

EXPECT_THAT(read_scn<int>(), Optional(100));
EXPECT_THAT(read_scn<int>(), Optional(101));
EXPECT_THAT(read_scanf<int>(), Optional(102));
EXPECT_THAT(read_scn<int>(), Optional(103));
EXPECT_THAT(read_cin<int>(), Optional(104));
EXPECT_THAT(read_scn<int>(), Optional(105));

EXPECT_EQ(read_scn<int>(), std::nullopt);
EXPECT_THAT(read_scn<std::string>(), Optional("aaa"s));

EXPECT_EQ(read_scn<int>(), std::nullopt);
EXPECT_THAT(read_scanf<std::string>(), Optional("bbb"s));

EXPECT_EQ(read_scn<int>(), std::nullopt);
EXPECT_THAT(read_cin<std::string>(), Optional("ccc"s));
}
1 change: 1 addition & 0 deletions tests/unittests/stdin_test_input.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
100 101 102 103 104 105 aaa bbb ccc
3 changes: 3 additions & 0 deletions tests/unittests/stdin_test_runner.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/usr/bin/env bash

"$(dirname "$0")/scn_stdin_test" < "$(dirname "$0")/stdin_test_input.txt"

0 comments on commit 5f316d3

Please sign in to comment.