Skip to content

Commit 16752ca

Browse files
ChALkeRnkreeger
authored andcommitted
file_system: group promisify, avoid relying on in-advance fs.exists checks (tensorflow#112)
* file_system: move promisify to top No need to call promisify() per every method call, we can do that once instead and save the result. * file_system: avoid relying on in-advance fs.exists check in dir creation This avoids possible race conditions, e.g. when more than one parallel async .save() operation is called (from a single or several processes) sharing a common prefix which does not yet exist. Instead, createOrVerifyDirectory() now just tries creating the dir, and if that fails -- checks _why_ that failed, if it is anything other than the directory already existing -- throws an error, if it's the dir already existing -- skips that and continues. Refs: https://nodejs.org/api/fs.html#fs_fs_exists_path_callback * file_system: remove remaining fs.exists calls Instead, use fs.stat and fs.readFile directly and let them throw if the file in question does not exist, and format the error in a way how we want using a small helper function.
1 parent 4f997f7 commit 16752ca

File tree

1 file changed

+33
-22
lines changed

1 file changed

+33
-22
lines changed

src/io/file_system.ts

+33-22
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,26 @@ import * as fs from 'fs';
2020
import {dirname, join, resolve} from 'path';
2121
import {promisify} from 'util';
2222

23+
const stat = promisify(fs.stat);
24+
const writeFile = promisify(fs.writeFile);
25+
const readFile = promisify(fs.readFile);
26+
const mkdir = promisify(fs.mkdir);
27+
2328
// tslint:disable-next-line:max-line-length
2429
import {getModelArtifactsInfoForJSON, toArrayBuffer} from './io_utils';
2530

31+
function doesNotExistHandler(name: string):
32+
(e: NodeJS.ErrnoException) => never {
33+
return e => {
34+
switch (e.code) {
35+
case 'ENOENT':
36+
throw new Error(`${name} ${e.path} does not exist: loading failed`);
37+
default:
38+
throw e;
39+
}
40+
};
41+
}
42+
2643
export class NodeFileSystem implements tfc.io.IOHandler {
2744
static readonly URL_SCHEME = 'file://';
2845

@@ -81,7 +98,6 @@ export class NodeFileSystem implements tfc.io.IOHandler {
8198
weightsManifest,
8299
};
83100
const modelJSONPath = join(this.path, this.MODEL_JSON_FILENAME);
84-
const writeFile = promisify(fs.writeFile);
85101
await writeFile(modelJSONPath, JSON.stringify(modelJSON), 'utf8');
86102
await writeFile(
87103
weightsBinPath, Buffer.from(modelArtifacts.weightData), 'binary');
@@ -102,16 +118,11 @@ export class NodeFileSystem implements tfc.io.IOHandler {
102118
// https://github.com/tensorflow/tfjs/issues/343
103119
}
104120

105-
const exists = promisify(fs.exists);
106-
if (!await exists(this.path)) {
107-
throw new Error(`Path ${this.path} does not exist: loading failed.`);
108-
}
121+
const info = await stat(this.path).catch(doesNotExistHandler('Path'));
109122

110123
// `this.path` can be either a directory or a file. If it is a file, assume
111124
// it is model.json file.
112-
const stat = promisify(fs.stat);
113-
if ((await stat(this.path)).isFile()) {
114-
const readFile = promisify(fs.readFile);
125+
if (info.isFile()) {
115126
const modelJSON = JSON.parse(await readFile(this.path, 'utf8'));
116127

117128
const modelArtifacts: tfc.io.ModelArtifacts = {
@@ -124,11 +135,8 @@ export class NodeFileSystem implements tfc.io.IOHandler {
124135
for (const group of modelJSON.weightsManifest) {
125136
for (const path of group.paths) {
126137
const weightFilePath = join(dirName, path);
127-
if (!await exists(weightFilePath)) {
128-
throw new Error(`Weight file ${
129-
weightFilePath} does not exist: loading failed`);
130-
}
131-
const buffer = await readFile(weightFilePath);
138+
const buffer = await readFile(weightFilePath)
139+
.catch(doesNotExistHandler('Weight file'));
132140
buffers.push(buffer);
133141
}
134142
weightSpecs.push(...group.weights);
@@ -149,18 +157,21 @@ export class NodeFileSystem implements tfc.io.IOHandler {
149157
* that the path exists as a directory.
150158
*/
151159
protected async createOrVerifyDirectory() {
152-
for (const path of Array.isArray(this.path) ? this.path : [this.path]) {
153-
const exists = promisify(fs.exists);
154-
const stat = promisify(fs.stat);
155-
if (await exists(path)) {
156-
if ((await stat(path)).isFile()) {
157-
throw new Error(
160+
const paths = Array.isArray(this.path) ? this.path : [this.path];
161+
for (const path of paths) {
162+
try {
163+
await mkdir(path);
164+
} catch (e) {
165+
if (e.code === 'EEXIST') {
166+
if ((await stat(path)).isFile()) {
167+
throw new Error(
158168
`Path ${path} exists as a file. The path must be ` +
159169
`nonexistent or point to a directory.`);
170+
}
171+
// else continue, the directory exists
172+
} else {
173+
throw e;
160174
}
161-
} else {
162-
const mkdir = promisify(fs.mkdir);
163-
await mkdir(path);
164175
}
165176
}
166177
}

0 commit comments

Comments
 (0)