Skip to content

Commit

Permalink
Fix runtime cache racing condition (#4184)
Browse files Browse the repository at this point in the history
  • Loading branch information
ilia-kebets-sonarsource authored Sep 20, 2023
1 parent c0917ba commit 814732f
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
"angular.js:test/helpers/support.js": [
16
],
"angular.js:test/ng/compileSpec.fixture.js": [
688
],
"angular.js:test/ng/compileSpec.js": [
688
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,25 +94,15 @@
"javascript-test-sources:src/ace/src/mouse/touch_handler.js": [
59
],
"javascript-test-sources:src/ace/src/scrollbar.js": [
125,
130,
223
],
"javascript-test-sources:src/ace/src/scrollbar_test.js": [
13
],
"javascript-test-sources:src/ace/src/snippets.js": [
270
],
"javascript-test-sources:src/ace/src/test/all_browser.js": [
11,
116
],
"javascript-test-sources:src/ace/src/virtual_renderer.js": [
1159,
1167
],
"javascript-test-sources:src/ace/static.js": [
33
],
Expand All @@ -139,15 +129,6 @@
123,
124
],
"javascript-test-sources:src/ecmascript6/Ghost/core/server/models/post.js": [
329
],
"javascript-test-sources:src/ecmascript6/Ghost/core/server/models/tag.js": [
68
],
"javascript-test-sources:src/ecmascript6/Ghost/core/server/models/user.js": [
184
],
"javascript-test-sources:src/ecmascript6/ecmascript6-today/7-rest-params/js/rest-spread.js": [
25
],
Expand All @@ -160,8 +141,6 @@
],
"javascript-test-sources:src/ecmascript6/router/third_party/brick/brick-1.0.1.byob.js": [
337,
572,
575,
877,
1412,
1637,
Expand Down
2 changes: 2 additions & 0 deletions its/ruling/src/test/expected/js/p5.js/javascript-S1874.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"p5.js:docs/yuidoc-p5-theme/assets/js/reference.js": [
2433,
2433,
2535,
3895,
4316,
4316,
4503,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ static void runRulingTest(
.setTestDirs(testDir)
.setSourceEncoding("utf-8")
.setScannerVersion(SCANNER_VERSION)
.setDebugLogs(true)
.setProperty(
"sonar.lits.dump.old",
FileLocation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE;

import java.io.BufferedInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
Expand All @@ -49,6 +50,7 @@ public class EmbeddedNode {

private static final String DEPLOY_LOCATION = Path.of(".sonar", "js", "node-runtime").toString();
public static final String VERSION_FILENAME = "version.txt";
private static final long EXTRACTION_LOCK_WAIT_TIME_MILLIS = 10000;
private static final Logger LOG = Loggers.get(EmbeddedNode.class);
private Path deployLocation;
private final Platform platform;
Expand Down Expand Up @@ -126,14 +128,14 @@ private static boolean isARM64(Environment env) {

public EmbeddedNode(Environment env) {
this.platform = Platform.detect(env);
this.deployLocation = getPluginCache(env.getUserHome());
this.deployLocation = runtimeCachePathFrom(env.getUserHome());
this.env = env;
}

/**
* @return a path to `DEPLOY_LOCATION` from the given `baseDir`
*/
private static Path getPluginCache(String baseDir) {
private static Path runtimeCachePathFrom(String baseDir) {
return Path.of(baseDir).resolve(DEPLOY_LOCATION);
}

Expand All @@ -158,20 +160,16 @@ public void deploy() throws IOException {
return;
}

var targetArchive = deployLocation.resolve(platform.binary() + ".xz");
var targetDirectory = targetArchive.getParent();
var targetRuntime = deployLocation.resolve(platform.binary());
var targetDirectory = targetRuntime.getParent();
var targetVersion = targetDirectory.resolve(VERSION_FILENAME);
// we assume that since the archive exists, the version file must as well
var versionIs = getClass().getResourceAsStream(platform.versionPathInJar());

if (!Files.exists(targetVersion) || isDifferent(versionIs, targetVersion)) {
LOG.debug("Copy embedded node to {}", targetArchive);
Files.createDirectories(targetDirectory);
Files.copy(is, targetArchive, REPLACE_EXISTING);
extract(targetArchive);
Files.copy(versionIs, deployLocation.resolve(VERSION_FILENAME), REPLACE_EXISTING);
} else {
if (Files.exists(targetVersion) && !isDifferent(versionIs, targetVersion)) {
LOG.debug("Skipping node deploy. Deployed node has latest version.");
} else {
extractWithLocking(is, versionIs, targetRuntime, targetDirectory);
}

isAvailable = true;
Expand All @@ -193,24 +191,64 @@ private static boolean isDifferent(InputStream newVersionIs, Path currentVersion
}

/**
* Expects a path to a xz-compressed file ending in `.xz` like `node.xz` and
* extracts it into the same place as `node`.
* Creates the `targetDirectory` and extracts the `source` to `targetRuntime`
* Writes the version from `versionIs` to `targetDirectory`/VERSION_FILENAME
*
* @param source
* @param versionIs
* @param targetRuntime
* @param targetDirectory
* @throws IOException
*/
private void extractWithLocking(
InputStream source,
InputStream versionIs,
Path targetRuntime,
Path targetDirectory
) throws IOException {
var targetLockFile = targetDirectory.resolve("lockfile");
Files.createDirectories(targetDirectory);
try (
var fos = new FileOutputStream(targetLockFile.toString());
var channel = fos.getChannel()
) {
var lock = channel.tryLock();
if (lock != null) {
try {
LOG.debug("Locked file: " + targetRuntime + " using lock " + lock);
extract(source, targetRuntime);
Files.copy(versionIs, deployLocation.resolve(VERSION_FILENAME), REPLACE_EXISTING);
} finally {
lock.release();
}
} else {
try {
LOG.debug("Waiting");
Thread.sleep(EXTRACTION_LOCK_WAIT_TIME_MILLIS);
} catch (InterruptedException e) {
LOG.warn("Interrupted while waiting for another process to extract the node runtime");
Thread.currentThread().interrupt();
}
}
}
}

/**
* Expects an InputStream to a xz-compressed file ending in `.xz` like `node.xz` and
* extracts it into the the given target Path.
* <p>
* Skips extraction if target file already exists.
*
* @param source Path for the file to extract
* @throws IOException
*/
private void extract(Path source) throws IOException {
var sourceAsString = source.toString();
var target = Path.of(sourceAsString.substring(0, sourceAsString.length() - 3));
LOG.debug("Decompressing " + source.toAbsolutePath() + " into " + target);
private void extract(InputStream source, Path target) throws IOException {
try (
var is = Files.newInputStream(source);
var stream = new BufferedInputStream(is);
var stream = new BufferedInputStream(source);
var archive = new XZInputStream(stream);
var os = Files.newOutputStream(target);
) {
LOG.debug("Extracting embedded node to {}", target);
int nextBytes;
byte[] buf = new byte[8 * 1024 * 1024];
while ((nextBytes = archive.read(buf)) > -1) {
Expand Down

0 comments on commit 814732f

Please sign in to comment.