From ad8b3442983e0370e95573dfc68d3bcbba9565f4 Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Mon, 29 Apr 2019 19:29:27 -0700 Subject: [PATCH] tsan fixes on ubuntu xenial (what travis run) --- Dockerfile | 43 +-------------------------- docker/Dockerfile.debian | 52 +++++++++++++++++++++++++++++++++ docker/Dockerfile.fedora | 42 ++++++++++++++++++++++++++ docker/Dockerfile.ubuntu_xenial | 24 +++++++++++++++ ixwebsocket/IXDNSLookup.cpp | 37 ++++++++++++++--------- ixwebsocket/IXDNSLookup.h | 15 ++++++++-- 6 files changed, 156 insertions(+), 57 deletions(-) mode change 100644 => 120000 Dockerfile create mode 100644 docker/Dockerfile.debian create mode 100644 docker/Dockerfile.fedora create mode 100644 docker/Dockerfile.ubuntu_xenial diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index ac2f178e..00000000 --- a/Dockerfile +++ /dev/null @@ -1,42 +0,0 @@ -FROM fedora:30 as build - -RUN yum install -y gcc-g++ -RUN yum install -y cmake -RUN yum install -y make -RUN yum install -y openssl-devel - -RUN yum install -y wget -RUN mkdir -p /tmp/cmake -WORKDIR /tmp/cmake -RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz -RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz - -ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin -ENV PATH="${CMAKE_BIN_PATH}:${PATH}" - -RUN yum install -y python -RUN yum install -y libtsan - -COPY . . -# RUN ["make", "test"] -RUN ["make"] - -# Runtime -FROM fedora:30 as runtime - -RUN yum install -y libtsan - -RUN groupadd app && useradd -g app app -COPY --chown=app:app --from=build /usr/local/bin/ws /usr/local/bin/ws -RUN chmod +x /usr/local/bin/ws -RUN ldd /usr/local/bin/ws - -# Now run in usermode -USER app -WORKDIR /home/app - -COPY --chown=app:app ws/snake/appsConfig.json . -COPY --chown=app:app ws/cobraMetricsSample.json . - -ENTRYPOINT ["ws"] -CMD ["--help"] diff --git a/Dockerfile b/Dockerfile new file mode 120000 index 00000000..d81c7222 --- /dev/null +++ b/Dockerfile @@ -0,0 +1 @@ +docker/Dockerfile.fedora \ No newline at end of file diff --git a/docker/Dockerfile.debian b/docker/Dockerfile.debian new file mode 100644 index 00000000..d24abf9e --- /dev/null +++ b/docker/Dockerfile.debian @@ -0,0 +1,52 @@ +# Build time +FROM debian:buster as build + +ENV DEBIAN_FRONTEND noninteractive +RUN apt-get update +RUN apt-get -y install wget +RUN mkdir -p /tmp/cmake +WORKDIR /tmp/cmake +RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz +RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz + +RUN apt-get -y install g++ +RUN apt-get -y install libssl-dev +RUN apt-get -y install libz-dev +RUN apt-get -y install make + +COPY . . + +ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin +ENV PATH="${CMAKE_BIN_PATH}:${PATH}" + +RUN ["make"] + +# Runtime +FROM debian:buster as runtime + +ENV DEBIAN_FRONTEND noninteractive +RUN apt-get update +# Runtime +RUN apt-get install -y libssl1.1 +RUN apt-get install -y ca-certificates +RUN ["update-ca-certificates"] + +# Debugging +RUN apt-get install -y strace +RUN apt-get install -y procps +RUN apt-get install -y htop + +RUN adduser --disabled-password --gecos '' app +COPY --chown=app:app --from=build /usr/local/bin/ws /usr/local/bin/ws +RUN chmod +x /usr/local/bin/ws +RUN ldd /usr/local/bin/ws + +# Now run in usermode +USER app +WORKDIR /home/app + +COPY --chown=app:app ws/snake/appsConfig.json . +COPY --chown=app:app ws/cobraMetricsSample.json . + +ENTRYPOINT ["ws"] +CMD ["--help"] diff --git a/docker/Dockerfile.fedora b/docker/Dockerfile.fedora new file mode 100644 index 00000000..ac2f178e --- /dev/null +++ b/docker/Dockerfile.fedora @@ -0,0 +1,42 @@ +FROM fedora:30 as build + +RUN yum install -y gcc-g++ +RUN yum install -y cmake +RUN yum install -y make +RUN yum install -y openssl-devel + +RUN yum install -y wget +RUN mkdir -p /tmp/cmake +WORKDIR /tmp/cmake +RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz +RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz + +ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin +ENV PATH="${CMAKE_BIN_PATH}:${PATH}" + +RUN yum install -y python +RUN yum install -y libtsan + +COPY . . +# RUN ["make", "test"] +RUN ["make"] + +# Runtime +FROM fedora:30 as runtime + +RUN yum install -y libtsan + +RUN groupadd app && useradd -g app app +COPY --chown=app:app --from=build /usr/local/bin/ws /usr/local/bin/ws +RUN chmod +x /usr/local/bin/ws +RUN ldd /usr/local/bin/ws + +# Now run in usermode +USER app +WORKDIR /home/app + +COPY --chown=app:app ws/snake/appsConfig.json . +COPY --chown=app:app ws/cobraMetricsSample.json . + +ENTRYPOINT ["ws"] +CMD ["--help"] diff --git a/docker/Dockerfile.ubuntu_xenial b/docker/Dockerfile.ubuntu_xenial new file mode 100644 index 00000000..e24fe498 --- /dev/null +++ b/docker/Dockerfile.ubuntu_xenial @@ -0,0 +1,24 @@ +# Build time +FROM ubuntu:xenial as build + +ENV DEBIAN_FRONTEND noninteractive +RUN apt-get update +RUN apt-get -y install wget +RUN mkdir -p /tmp/cmake +WORKDIR /tmp/cmake +RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz +RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz + +RUN apt-get -y install g++ +RUN apt-get -y install libssl-dev +RUN apt-get -y install libz-dev +RUN apt-get -y install make +RUN apt-get -y install python + +COPY . . + +ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin +ENV PATH="${CMAKE_BIN_PATH}:${PATH}" + +# RUN ["make"] +RUN ["make", "test"] diff --git a/ixwebsocket/IXDNSLookup.cpp b/ixwebsocket/IXDNSLookup.cpp index f0bd635f..40f91097 100644 --- a/ixwebsocket/IXDNSLookup.cpp +++ b/ixwebsocket/IXDNSLookup.cpp @@ -19,25 +19,24 @@ namespace ix std::mutex DNSLookup::_activeJobsMutex; DNSLookup::DNSLookup(const std::string& hostname, int port, int64_t wait) : - _hostname(hostname), _port(port), _wait(wait), _res(nullptr), _done(false), _id(_nextId++) { - ; + setHostname(hostname); } DNSLookup::~DNSLookup() { // Remove this job from the active jobs list - std::unique_lock lock(_activeJobsMutex); + std::lock_guard lock(_activeJobsMutex); _activeJobs.erase(_id); } // we want hostname to be copied, not passed as a const reference - struct addrinfo* DNSLookup::getAddrInfo(std::string hostname, + struct addrinfo* DNSLookup::getAddrInfo(const std::string& hostname, int port, std::string& errMsg) { @@ -80,7 +79,7 @@ namespace ix return nullptr; } - return getAddrInfo(_hostname, _port, errMsg); + return getAddrInfo(getHostname(), _port, errMsg); } struct addrinfo* DNSLookup::resolveAsync(std::string& errMsg, @@ -98,7 +97,7 @@ namespace ix // Record job in the active Job set { - std::unique_lock lock(_activeJobsMutex); + std::lock_guard lock(_activeJobsMutex); _activeJobs.insert(_id); } @@ -106,7 +105,7 @@ namespace ix // Good resource on thread forced termination // https://www.bo-yang.net/2017/11/19/cpp-kill-detached-thread // - _thread = std::thread(&DNSLookup::run, this, _id, _hostname, _port); + _thread = std::thread(&DNSLookup::run, this, _id, getHostname(), _port); _thread.detach(); std::unique_lock lock(_conditionVariableMutex); @@ -140,7 +139,7 @@ namespace ix return getRes(); } - void DNSLookup::run(uint64_t id, std::string hostname, int port) // thread runner + void DNSLookup::run(uint64_t id, const std::string& hostname, int port) // thread runner { // We don't want to read or write into members variables of an object that could be // gone, so we use temporary variables (res) or we pass in by copy everything that @@ -151,7 +150,7 @@ namespace ix // if this isn't an active job, and the control thread is gone // there is nothing to do, and we don't want to touch the defunct // object data structure such as _errMsg or _condition - std::unique_lock lock(_activeJobsMutex); + std::lock_guard lock(_activeJobsMutex); if (_activeJobs.count(id) == 0) { return; @@ -165,27 +164,39 @@ namespace ix _done = true; } + void DNSLookup::setHostname(const std::string& hostname) + { + std::lock_guard lock(_hostnameMutex); + _hostname = hostname; + } + + const std::string& DNSLookup::getHostname() + { + std::lock_guard lock(_hostnameMutex); + return _hostname; + } + void DNSLookup::setErrMsg(const std::string& errMsg) { - std::unique_lock lock(_errMsgMutex); + std::lock_guard lock(_errMsgMutex); _errMsg = errMsg; } const std::string& DNSLookup::getErrMsg() { - std::unique_lock lock(_errMsgMutex); + std::lock_guard lock(_errMsgMutex); return _errMsg; } void DNSLookup::setRes(struct addrinfo* addr) { - std::unique_lock lock(_resMutex); + std::lock_guard lock(_resMutex); _res = addr; } struct addrinfo* DNSLookup::getRes() { - std::unique_lock lock(_resMutex); + std::lock_guard lock(_resMutex); return _res; } } diff --git a/ixwebsocket/IXDNSLookup.h b/ixwebsocket/IXDNSLookup.h index fe3ae454..f5643ed8 100644 --- a/ixwebsocket/IXDNSLookup.h +++ b/ixwebsocket/IXDNSLookup.h @@ -39,11 +39,20 @@ namespace ix struct addrinfo* resolveBlocking(std::string& errMsg, const CancellationRequest& isCancellationRequested); - static struct addrinfo* getAddrInfo(std::string hostname, + static struct addrinfo* getAddrInfo(const std::string& hostname, int port, std::string& errMsg); - void run(uint64_t id, std::string hostname, int port); // thread runner + void run(uint64_t id, const std::string& hostname, int port); // thread runner + + void setHostname(const std::string& hostname); + const std::string& getHostname(); + + void setErrMsg(const std::string& errMsg); + const std::string& getErrMsg(); + + void setRes(struct addrinfo* addr); + struct addrinfo* getRes(); void setErrMsg(const std::string& errMsg); const std::string& getErrMsg(); @@ -52,7 +61,9 @@ namespace ix struct addrinfo* getRes(); std::string _hostname; + std::mutex _hostnameMutex; int _port; + int64_t _wait; struct addrinfo* _res;