Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use-after free + Fix building on Windows + prebuild for Node, Electron or on x86 + add debug build + fix the tests and ci #444

Merged
merged 19 commits into from
Apr 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .mocharc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict'

module.exports = {
require: ['ts-node/register', 'choma'],
spec: ['test/unit/*-test.ts', 'test/unit/compat/*-test.{ts,js}'],
"expose-gc": true,
"experimental-worker": true,
recursive: true,
}
37 changes: 20 additions & 17 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,59 +1,62 @@
language: node_js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we favor GitHub Actions over Travis for the 6.x branch moving forward?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can. I prefer to do this in the follow-up pull requests to keep things simpler.

cache: yarn
dist: bionic
python:
- 3.6

jobs:
include:

## TEST STAGE

- os: linux
node_js: "10.2"
node_js: "14"
env: ZMQ_DRAFT=true

- os: linux
node_js: "10.16"
node_js: "14"
env: ZMQ_DRAFT=true INCLUDE_COMPAT_TESTS=true

- os: linux
env: ALPINE_CHROOT=3.10 ZMQ_DRAFT=true INCLUDE_COMPAT_TESTS=true
sudo: required
node_js: "14"

- os: osx
osx_image: xcode10
env: ZMQ_DRAFT=true
node_js: "10.16"
node_js: "14"

- os: windows
node_js: "10.16"
node_js: "14"
# https://travis-ci.community/t/build-doesnt-finish-after-completing-tests/288
env: ZMQ_DRAFT=true YARN_GPG=no

- os: windows
node_js: "10.16/x86"
node_js: "14/x86"
# https://travis-ci.community/t/build-doesnt-finish-after-completing-tests/288
env: ZMQ_DRAFT=true YARN_GPG=no

# Test shared libraries on Linux and macOS.
- os: linux
node_js: "10.16"
node_js: "14"
env: ZMQ_SHARED=true
addons: {apt: {packages: libzmq3-dev}}

- os: osx
osx_image: xcode10
node_js: "10.16"
node_js: "14"
env: ZMQ_SHARED=true
addons: {homebrew: {packages: zeromq, update: true}}

# Test older versions of ZMQ.
- os: linux
node_js: "10.16"
node_js: "14"
env: ZMQ_VERSION=4.2.4

# Test recent Node versions.
- os: linux
node_js: "12"
node_js: "14"
# Skip GC tests due to https://github.com/node-ffi-napi/weak-napi/issues/16
env: ZMQ_DRAFT=true SKIP_GC_TESTS=true INCLUDE_COMPAT_TESTS=true

Expand All @@ -67,15 +70,15 @@ jobs:
# This test ensures the delayed resolution of read/write promises is correct
# by disabling immediate resolution (which happens 99% of the time) entirely.
- os: linux
node_js: "10.16"
node_js: "14"
env: ZMQ_NO_SYNC_RESOLVE=true ZMQ_DRAFT=true INCLUDE_COMPAT_TESTS=true NODE_NO_WARNINGS=1

## PREBUILD STAGE

- stage: prebuild
os: linux
env: ARCHIVE_SUFFIX=-x64
node_js: "10.16"
node_js: "14"
script: script/ci/prebuild.sh

- stage: prebuild
Expand All @@ -86,34 +89,34 @@ jobs:

- stage: prebuild
os: linux
node_js: "10.16"
node_js: "14"
env: ARCH=arm TRIPLE=arm-linux-gnueabihf GCC=8 ARCHIVE_SUFFIX=-armv7
addons: {apt: {packages: [gcc-8-arm-linux-gnueabihf, g++-8-arm-linux-gnueabihf]}}
script: script/ci/prebuild.sh

- stage: prebuild
os: linux
node_js: "10.16"
node_js: "14"
env: ARCH=arm64 TRIPLE=aarch64-linux-gnu GCC=8 ARCHIVE_SUFFIX=-armv8
addons: {apt: {packages: [gcc-8-aarch64-linux-gnu, g++-8-aarch64-linux-gnu]}}
script: script/ci/prebuild.sh

- stage: prebuild
os: osx
osx_image: xcode10
node_js: "10.16"
node_js: "14"
script: script/ci/prebuild.sh

- stage: prebuild
os: windows
node_js: "10.16"
node_js: "14"
# https://travis-ci.community/t/build-doesnt-finish-after-completing-tests/288
env: YARN_GPG=no ARCHIVE_SUFFIX=-x64
script: script/ci/prebuild.sh

- stage: prebuild
os: windows
node_js: "10.16/x86"
node_js: "14/x86"
# https://travis-ci.community/t/build-doesnt-finish-after-completing-tests/288
env: YARN_GPG=no ARCHIVE_SUFFIX=-x86
script: script/ci/prebuild.sh
Expand All @@ -122,7 +125,7 @@ jobs:

- stage: publish
os: linux
node_js: "10.16"
node_js: "14"
env: IGNORE_SCRIPTS=true
script: script/ci/package.sh

Expand Down
59 changes: 46 additions & 13 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
'action_name': 'build_libzmq',
'inputs': ['package.json'],
'outputs': ['libzmq/lib'],
'action': ['sh', '<(PRODUCT_DIR)/../../script/build.sh'],
'action': ['sh', '<(PRODUCT_DIR)/../../script/build.sh', '<(CONFIGURATION_NAME)'],
}],
}],
],
Expand All @@ -38,11 +38,11 @@
'include_dirs': [
"vendor",
'<(PRODUCT_DIR)/../libzmq/include',
"<!@(node -p \"require('node-addon-api').include\")"
],

'defines': [
'NAPI_VERSION=3',
'NAPI_DISABLE_CPP_EXCEPTIONS',
'ZMQ_STATIC',
],

Expand Down Expand Up @@ -72,7 +72,6 @@
}],

['OS == "win"', {
'msbuild_toolset': 'v141',
'libraries': [
'<(PRODUCT_DIR)/../libzmq/lib/libzmq',
'ws2_32.lib',
Expand All @@ -85,6 +84,7 @@

'configurations': {
'Debug': {
'defines': ['NAPI_CPP_EXCEPTIONS', 'DEBUG', '_DEBUG'],
'conditions': [
['OS == "linux" or OS == "freebsd" or OS == "openbsd" or OS == "solaris"', {
'cflags_cc!': [
Expand Down Expand Up @@ -113,22 +113,55 @@

['OS == "win"', {
'msvs_settings': {
'VCCLCompilerTool': {
# 0 - MultiThreaded (/MT)
# 1 - MultiThreadedDebug (/MTd)
# 2 - MultiThreadedDLL (/MD)
# 3 - MultiThreadedDebugDLL (/MDd)
'RuntimeLibrary': 3,
'AdditionalOptions': [
'-std:c++17',
],
},
'conditions': [
['"<@(sanitizers)" != "true"', {
# without sanitizer
'VCCLCompilerTool': {
'ExceptionHandling': 2, # /EHsc
# 0 - MultiThreaded (/MT)
# 1 - MultiThreadedDebug (/MTd)
# 2 - MultiThreadedDLL (/MD)
# 3 - MultiThreadedDebugDLL (/MDd)
'RuntimeLibrary': 3,
'AdditionalOptions': [
'-std:c++17',
"/DEBUG",
],
},
'VCLinkerTool': {
'BasicRuntimeChecks': 3, # /RTC1
},
}, {
# with sanitizer
# Build with `node-gyp rebuild --debug --sanitizers='true'`
# Make sure to add the followings (or what your MSVC use) to the PATH and run `refreshenv`.
# # C:/Program Files (x86)/Microsoft Visual Studio/2019/Preview/VC/Tools/MSVC/14.29.29917/lib/x64/
# # C:/Program Files (x86)/Microsoft Visual Studio/2019/Preview/VC/Tools/MSVC/14.29.29917/bin/Hostx64/x64/
'VCCLCompilerTool': {
'ExceptionHandling': 2, # /EHsc
'RuntimeLibrary': 3,
"DebugInformationFormat": "ProgramDatabase", # /Zi
'AdditionalOptions': [
'-std:c++17',
"/DEBUG",
"/fsanitize=address",
],
},
'VCLinkerTool': {
'BasicRuntimeChecks': 0, # not supported with fsanitize
"LinkIncremental": "NO", # /INCREMENTAL:NO
},
}],
],
},
}],
],
},

'Release': {
'defines': [
'NAPI_DISABLE_CPP_EXCEPTIONS',
],
'conditions': [
['OS == "linux" or OS == "freebsd" or OS == "openbsd" or OS == "solaris"', {
'cflags_cc!': [
Expand Down
10 changes: 6 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"@types/mocha": ">= 5.2",
"@types/node": ">= 8.0",
"@types/semver": ">= 0",
"@types/weak-napi": "^2.0.0",
"@typescript-eslint/eslint-plugin": "^2.9.0",
"@typescript-eslint/parser": "^2.9.0",
"benchmark": ">= 0",
Expand All @@ -38,16 +39,16 @@
"fs-extra": "^8.1.0",
"gunzip-maybe": "^1.4.1",
"mocha": ">= 4.0",
"node-addon-api": "nodejs/node-addon-api",
"node-addon-api": "^3.1.0",
"node-fetch": "^2.6.0",
"prebuildify": "^3.0",
"prebuildify": "^4.1.2",
"prettier": "^1.19.1",
"semver": ">= 0",
"tar-fs": "^2.0.0",
"ts-morph": "^7.0.0",
"ts-node": ">= 7",
"typescript": ">= 3.6",
"weak-napi": ">= 1.0"
"weak-napi": "^2.0.2"
},
"engines": {
"node": ">= 10.2"
Expand All @@ -74,7 +75,8 @@
"install": "node-gyp-build",
"ci:compile": "tsc --project tsconfig-build.json && node script/ci/downlevel-dts.js",
"ci:doc": "typedoc --out docs --name zeromq.js --excludeProtected --excludePrivate --excludeNotExported --excludeExternals --externalPattern 'src/+(draft|native|compat).ts' --tsconfig tsconfig-build.json --mode file",
"ci:prebuild": "prebuildify --napi --strip",
"ci:prebuild": "npm run prebuildify",
"prebuildify": "prebuildify --napi -t 12.0.0 -t electron@9.4.4 --strip",
"dev:build": "rm -f vendor/* && touch vendor/.gitkeep && cp node_modules/node-addon-api/{*.h,LICENSE.md} vendor && prebuildify --napi --build-from-source --debug",
"dev:test": "tsc --project tsconfig-build.json && node script/ci/downlevel-dts.js && mocha && script/format.sh && rm -f tmp/*",
"dev:bench": "node --expose-gc test/bench"
Expand Down
27 changes: 8 additions & 19 deletions script/build.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/sh
set -e

ZMQ_VERSION=${ZMQ_VERSION:-"4.3.2"}
ZMQ_VERSION=${ZMQ_VERSION:-"4.3.4"}

SRC_URL="https://github.com/zeromq/libzmq/releases/download/v${ZMQ_VERSION}/zeromq-${ZMQ_VERSION}.tar.gz"
SRC_DIR="zeromq-${ZMQ_VERSION}"
Expand All @@ -12,22 +12,10 @@ if [ -n "${WINDIR}" ]; then
# Working directory is NAPI temporary build directory.
PATH_PREFIX="${PWD}/libzmq"
ARTIFACT="${PATH_PREFIX}/lib/libzmq.lib"
CMAKE_GENERATOR="Visual Studio 15 2017"
TOOLSET_VERSION="141"

# In Travis CI, Node paths are:
# - C:\ProgramData\nvs\node\<version>\x64\node.exe
# - C:\ProgramData\nvs\node\<version>\x86\node.exe
if [[ "${NODE}" != *"x86"* ]]; then
# Target Windows x64 platform.
CMAKE_GENERATOR="${CMAKE_GENERATOR} Win64"
fi
else
# Working directory is project root.
PATH_PREFIX="${PWD}/build/libzmq"
ARTIFACT="${PATH_PREFIX}/lib/libzmq.a"
CMAKE_GENERATOR="Unix Makefiles"

export MACOSX_DEPLOYMENT_TARGET=10.9
fi

Expand Down Expand Up @@ -57,7 +45,7 @@ else
echo > "${SRC_DIR}/builds/cmake/Modules/ClangFormat.cmake"
fi

cmake -G "${CMAKE_GENERATOR}" \
cmake \
"${BUILD_OPTIONS}" \
-DCMAKE_INSTALL_PREFIX="${PATH_PREFIX}" \
-DCMAKE_INSTALL_LIBDIR=lib \
Expand All @@ -70,16 +58,17 @@ else
if [ -n "${WINDIR}" ]; then
cmake \
--build . \
--config Release \
--config $1 \
--target install \
-- -verbosity:Minimal -maxcpucount
mv \
"${PATH_PREFIX}/lib/libzmq-v${TOOLSET_VERSION}-mt-s-${ZMQ_VERSION//./_}.lib" \
"${PATH_PREFIX}/lib/libzmq.lib"

BuilFile=$(find $PATH_PREFIX/lib/*.lib -type f)
mv "$BuilFile" "${PATH_PREFIX}/lib/libzmq.lib"

else
cmake \
--build .\
--config Release \
--config $1 \
--target install \
-- -j5
fi
Expand Down
4 changes: 4 additions & 0 deletions script/ci/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ if [ -n "${ZMQ_NO_SYNC_RESOLVE}" ]; then
export npm_config_zmq_no_sync_resolve=true
fi

if [ -n "${ZMQ_SANITIZERS}" ]; then
export npm_config_sanitizers=true
fi

export npm_config_build_from_source=true

# Installing node-gyp globally facilitates calling it in various ways, not just
Expand Down
2 changes: 2 additions & 0 deletions src/socket.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/* Copyright (c) 2017-2019 Rolf Timmermans */

#define NOMINMAX // prevent minwindef.h from defining max macro in the debug build
#include "socket.h"
#include "context.h"
#include "incoming_msg.h"
Expand Down
5 changes: 0 additions & 5 deletions test/mocha.opts

This file was deleted.

4 changes: 2 additions & 2 deletions test/unit/socket-close-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
if (process.env.SKIP_GC_TESTS) this.skip()
this.slow(200)

const weak = require("weak-napi")
const weak = require("weak-napi") as typeof import("weak-napi")

let released = false
const task = async () => {
Expand Down Expand Up @@ -135,7 +135,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
if (process.env.SKIP_GC_TESTS) this.skip()
this.slow(200)

const weak = require("weak-napi")
const weak = require("weak-napi") as typeof import("weak-napi")

let released = false
const task = async () => {
Expand Down
Loading