Deadlock using ASIO in multiple threads

[Shane Loretz]

[Impact]

The version of ASIO shipped with Bionic (1.10.8-1) has a race condition that can cause a program using ASIO in multiple threads to hang forever.
The attached test case reproduces it in a single program, but it also can happen if two libraries are independently using ASIO and linked by the same executable.

The justification for fixing the issue in Bionic is the patch is obviously safe and is not being applied to a critical infrastructure package.

The attached patch was taken from a commit to the upstream repository.
It appears to change the order that some state gets cleaned up.

[Test Case]

Save the attached files asio_stall_example.cpp and 0001-Don-t-free-the-reactor-s-per-descriptor-state-until-.patch

# Download the source code and extract it
wget https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/asio/1:1.10.8-1/asio_1.10.8.orig.tar.bz2
tar -xjvf asio_1.10.8.orig.tar.bz2

# Build using the extracted headers
g++ -o example --std=c++11 -I asio-1.10.8/include/ -pthread asio_stall_example.cpp

# Loop until the program hangs (happens within 1 to 20 iterations)
while ./example ; do date ; done

# Apply the attached patch
cd asio-1.10.8
patch -p 2 --input=../0001-Don-t-free-the-reactor-s-per-descriptor-state-until-.patch

# Rebuild with patch and run example again.
# The loop runs for much longer, only hanging momentarily if all ports are blocked by a socket in TIME_WAIT
g++ -o example --std=c++11 -I asio-1.10.8/include/ -pthread asio_stall_example.cpp
while ./example ; do date ; done

[Regression Potential]

ASIO is shipped as a header only library, so the potential for a regression is limited to binaries built after this patch is applied.
The fix only adds a non-virtual member function to internal 'reactor' classes, so I don't think ABI breakage of downstream libraries is a concern.

Reactor classes are internal and chosen by typedef to the type `reactor` depending on preprocessor definitions.
If there exists a custom reactor class downstream then it would be possible to get a compile time error about missing the method cleanup_descriptor_data().
Someone implementing a custom reactor class would need to add a typedef of their class to 'reactor' and prevent ASIO from creating it's own typedef.
It could be done by defining the include guard ASIO_DETAIL_REACTOR_FWD_HPP prior to including asio.

[Other Info]

The attached file asio_stall_example.cpp was taken from this issue on github and minimized
https://github.com/chriskohlhoff/asio/issues/180

The attached patch was made by cloning upstream and running
git format-patch -1 669e6b8

The original reporter on github says the issue only occurs if compiler optimizations are turned on; however, I see it both with and without optimizations.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Example program reproducing issue, and a patch to fix it." seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.