diff options
| -rw-r--r-- | src/lib/j2k_encoder.cc | 10 | ||||
| -rw-r--r-- | src/lib/writer.cc | 21 | ||||
| -rw-r--r-- | src/lib/writer.h | 4 | ||||
| -rw-r--r-- | test/j2k_encoder_test.cc | 83 | ||||
| -rw-r--r-- | test/wscript | 1 |
5 files changed, 119 insertions, 0 deletions
diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index 7c9777c16..32d2fefc2 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -69,6 +69,16 @@ J2KEncoder::~J2KEncoder () { _server_found_connection.disconnect(); + /* One of our encoder threads may be waiting on Writer::write() to return, if that method + * is blocked with the writer queue full waiting for _full_condition. In that case, the + * attempt to terminate the encoder threads below (in terminate_threads()) will fail because + * the encoder thread waiting for ::write() will have interruption disabled. + * + * To work around that, make the writer into a zombie to unblock any pending write()s and + * not block on any future ones. + */ + _writer.zombify(); + boost::mutex::scoped_lock lm (_threads_mutex); terminate_threads (); } diff --git a/src/lib/writer.cc b/src/lib/writer.cc index fbe2d248d..7b9defd73 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -144,6 +144,10 @@ Writer::write (shared_ptr<const Data> encoded, Frame frame, Eyes eyes) { boost::mutex::scoped_lock lock (_state_mutex); + if (_zombie) { + return; + } + while (_queued_full_in_memory > _maximum_frames_in_memory) { /* There are too many full frames in memory; wake the main writer thread and wait until it sorts everything out */ @@ -377,6 +381,9 @@ try while (true) { boost::mutex::scoped_lock lock (_state_mutex); + if (_zombie) { + return; + } while (true) { @@ -1042,3 +1049,17 @@ Writer::write_hanging_text (ReelWriter& reel) } _hanging_texts = new_hanging_texts; } + + +/** Set the writer so that it has no queue and drops any pending or future requests to write images */ +void +Writer::zombify() +{ + boost::mutex::scoped_lock lock(_state_mutex); + + _queue.clear(); + _queued_full_in_memory = 0; + _zombie = true; + _full_condition.notify_all(); +} + diff --git a/src/lib/writer.h b/src/lib/writer.h index 6e7f5ca90..f0f1fe69a 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -127,6 +127,8 @@ public: void set_encoder_threads (int threads); + void zombify(); + private: friend struct ::writer_disambiguate_font_ids1; friend struct ::writer_disambiguate_font_ids2; @@ -229,6 +231,8 @@ private: }; std::vector<HangingText> _hanging_texts; + + bool _zombie = false; }; diff --git a/test/j2k_encoder_test.cc b/test/j2k_encoder_test.cc new file mode 100644 index 000000000..bc3bd97b2 --- /dev/null +++ b/test/j2k_encoder_test.cc @@ -0,0 +1,83 @@ +/* + Copyright (C) 2024 Carl Hetherington <cth@carlh.net> + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see <http://www.gnu.org/licenses/>. + +*/ + + +#include "lib/config.h" +#include "lib/cross.h" +#include "lib/image.h" +#include "lib/j2k_encoder.h" +#include "lib/player_video.h" +#include "lib/raw_image_proxy.h" +#include "lib/writer.h" +#include "test.h" +extern "C" { +#include <libavutil/pixfmt.h> +} +#include <boost/test/unit_test.hpp> + + +using std::make_shared; +using std::weak_ptr; +using boost::optional; + + +BOOST_AUTO_TEST_CASE(j2k_encoder_deadlock_test) +{ + ConfigRestorer cr; + + auto film = new_test_film2("j2k_encoder_deadlock_test"); + + /* Don't call ::start() on this Writer, so it can never write anything */ + Writer writer(film, {}); + writer.set_encoder_threads(4); + + /* We want to test the case where the writer queue fills, and this can't happen unless there + * are enough encoding threads (each of which will end up waiting for the writer to empty, + * which will never happen). + */ + Config::instance()->set_master_encoding_threads(4); + J2KEncoder encoder(film, writer); + encoder.begin(); + + for (int i = 0; i < 26; ++i) { + auto image = make_shared<Image>(AV_PIX_FMT_RGB24, dcp::Size(1998, 1080), Image::Alignment::PADDED); + auto image_proxy = make_shared<RawImageProxy>(image); + encoder.encode( + std::make_shared<PlayerVideo>( + image_proxy, + Crop(), + optional<double>(), + dcp::Size(1998, 1080), + dcp::Size(1998, 1080), + Eyes::BOTH, + Part::WHOLE, + optional<ColourConversion>(), + VideoRange::VIDEO, + weak_ptr<Content>(), + optional<Frame>(), + false + ), + {} + ); + } + + dcpomatic_sleep_seconds(10); +} + diff --git a/test/wscript b/test/wscript index bb66d1a5f..cd23badc0 100644 --- a/test/wscript +++ b/test/wscript @@ -112,6 +112,7 @@ def build(bld): interrupt_encoder_test.cc isdcf_name_test.cc j2k_bandwidth_test.cc + j2k_encoder_test.cc job_manager_test.cc kdm_cli_test.cc kdm_naming_test.cc |