Skip to content

Commit 34c4235

Browse files
committed
Fix synchronization in PosixResources
1 parent 15f06a7 commit 34c4235

File tree

1 file changed

+64
-50
lines changed

1 file changed

+64
-50
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PosixResources.java

Lines changed: 64 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public PosixResources() {
202202
inodes = new HashMap<>();
203203
}
204204

205-
@TruffleBoundary(allowInlining = true)
205+
@TruffleBoundary
206206
public void setEnv(Env env) {
207207
synchronized (files) {
208208
files.get(FD_STDIN).setNewChannel(env.in());
@@ -241,18 +241,19 @@ private void removeFD(int fd) throws IOException {
241241
}
242242
}
243243

244+
/**
245+
* ATTENTION: This method must be used in a synchronized block (sync on {@link #files}) until.
246+
*/
244247
@TruffleBoundary
245248
private void dupFD(int fd1, int fd2) {
246249
ChannelWrapper channelWrapper = files.getOrDefault(fd1, null);
247250
if (channelWrapper != null) {
248-
synchronized (files) {
249-
channelWrapper.cnt += 1;
250-
files.put(fd2, channelWrapper);
251-
}
251+
channelWrapper.cnt += 1;
252+
files.put(fd2, channelWrapper);
252253
}
253254
}
254255

255-
@TruffleBoundary(allowInlining = true)
256+
@TruffleBoundary
256257
public Channel getFileChannel(int fd, ValueProfile classProfile) {
257258
ChannelWrapper channelWrapper = files.getOrDefault(fd, null);
258259
if (channelWrapper != null) {
@@ -261,7 +262,7 @@ public Channel getFileChannel(int fd, ValueProfile classProfile) {
261262
return null;
262263
}
263264

264-
@TruffleBoundary(allowInlining = true)
265+
@TruffleBoundary
265266
public FileLock getFileLock(int fd) {
266267
ChannelWrapper channelWrapper = files.getOrDefault(fd, null);
267268
if (channelWrapper != null) {
@@ -270,15 +271,15 @@ public FileLock getFileLock(int fd) {
270271
return null;
271272
}
272273

273-
@TruffleBoundary(allowInlining = true)
274+
@TruffleBoundary
274275
public void setFileLock(int fd, FileLock lock) {
275276
ChannelWrapper channelWrapper = files.getOrDefault(fd, null);
276277
if (channelWrapper != null) {
277278
channelWrapper.lock = lock;
278279
}
279280
}
280281

281-
@TruffleBoundary(allowInlining = true)
282+
@TruffleBoundary
282283
public Channel getFileChannel(int fd) {
283284
ChannelWrapper channelWrapper = files.getOrDefault(fd, null);
284285
if (channelWrapper != null) {
@@ -301,7 +302,7 @@ public PSocket getSocket(int fd) {
301302
return null;
302303
}
303304

304-
@TruffleBoundary(allowInlining = true)
305+
@TruffleBoundary
305306
public void close(int fd) {
306307
try {
307308
removeFD(fd);
@@ -311,17 +312,19 @@ public void close(int fd) {
311312

312313
@TruffleBoundary
313314
public int openSocket(PSocket socket) {
314-
int fd = nextFreeFd();
315-
addFD(fd, socket);
316-
return fd;
315+
synchronized (files) {
316+
int fd = nextFreeFd();
317+
addFD(fd, socket);
318+
return fd;
319+
}
317320
}
318321

319322
@TruffleBoundary
320323
public void reopenSocket(PSocket socket, int fd) {
321324
addFD(fd, socket);
322325
}
323326

324-
@TruffleBoundary(allowInlining = true)
327+
@TruffleBoundary
325328
public void fdopen(int fd, Channel fc) {
326329
files.get(fd).channel = fc;
327330
}
@@ -333,78 +336,89 @@ public void fdopen(int fd, Channel fc) {
333336
* @param fc the newly created Channel
334337
* @return the file descriptor associated with the new Channel
335338
*/
336-
@TruffleBoundary(allowInlining = true)
339+
@TruffleBoundary
337340
public int open(TruffleFile path, Channel fc) {
338-
int fd = nextFreeFd();
339-
addFD(fd, fc, path.getAbsoluteFile().getPath());
340-
return fd;
341+
synchronized (files) {
342+
int fd = nextFreeFd();
343+
addFD(fd, fc, path.getAbsoluteFile().getPath());
344+
return fd;
345+
}
341346
}
342347

343-
@TruffleBoundary(allowInlining = true)
348+
@TruffleBoundary
344349
public int dup(int fd) {
345-
int dupFd = nextFreeFd();
346-
dupFD(fd, dupFd);
347-
return dupFd;
350+
synchronized (files) {
351+
int dupFd = nextFreeFd();
352+
dupFD(fd, dupFd);
353+
return dupFd;
354+
}
348355
}
349356

350-
@TruffleBoundary(allowInlining = true)
357+
@TruffleBoundary
351358
public int dup2(int fd, int fd2) throws IOException {
352-
removeFD(fd2);
353-
dupFD(fd, fd2);
354-
return fd2;
359+
synchronized (files) {
360+
removeFD(fd2);
361+
dupFD(fd, fd2);
362+
return fd2;
363+
}
355364
}
356365

357-
@TruffleBoundary(allowInlining = true)
366+
@TruffleBoundary
358367
public boolean fsync(int fd) {
359368
return files.getOrDefault(fd, null) != null;
360369
}
361370

362-
@TruffleBoundary(allowInlining = true)
371+
@TruffleBoundary
363372
public void ftruncate(int fd, long size) throws IOException {
364373
Channel channel = getFileChannel(fd);
365374
if (channel instanceof SeekableByteChannel) {
366375
((SeekableByteChannel) channel).truncate(size);
367376
}
368377
}
369378

370-
@TruffleBoundary(allowInlining = true)
379+
@TruffleBoundary
371380
public int[] pipe() throws IOException {
372-
Pipe pipe = Pipe.open();
373-
int readFD = nextFreeFd();
374-
addFD(readFD, pipe.source());
381+
synchronized (files) {
382+
Pipe pipe = Pipe.open();
383+
int readFD = nextFreeFd();
384+
addFD(readFD, pipe.source());
375385

376-
int writeFD = nextFreeFd();
377-
addFD(writeFD, pipe.sink());
386+
int writeFD = nextFreeFd();
387+
addFD(writeFD, pipe.sink());
378388

379-
return new int[]{readFD, writeFD};
389+
return new int[]{readFD, writeFD};
390+
}
380391
}
381392

382-
@TruffleBoundary(allowInlining = true)
393+
/**
394+
* ATTENTION: This method must be used in a synchronized block (sync on {@link #files}) until
395+
* the gained file descriptors are written to the map. Otherwise, concurrent threads may get the
396+
* same FDs.
397+
*/
398+
@TruffleBoundary
383399
private int nextFreeFd() {
384-
synchronized (files) {
385-
int fd1 = files.firstKey();
386-
for (int fd2 : files.keySet()) {
387-
if (fd2 == fd1) {
388-
continue;
389-
}
390-
if (fd2 - fd1 > 1) {
391-
return fd1 + 1;
392-
} else {
393-
fd1 = fd2;
394-
}
400+
int fd1 = files.firstKey();
401+
for (int fd2 : files.keySet()) {
402+
if (fd2 == fd1) {
403+
continue;
404+
}
405+
if (fd2 - fd1 > 1) {
406+
return fd1 + 1;
407+
} else {
408+
fd1 = fd2;
395409
}
396-
return files.lastKey() + 1;
397410
}
411+
return files.lastKey() + 1;
398412
}
399413

400-
@TruffleBoundary(allowInlining = true)
414+
@TruffleBoundary
401415
public int registerChild(Process child) {
402416
int pid = nextFreePid();
403417
children.set(pid, child);
404418
return pid;
405419
}
406420

407-
@TruffleBoundary(allowInlining = true)
421+
@TruffleBoundary
408422
private int nextFreePid() {
409423
synchronized (children) {
410424
for (int i = 0; i < children.size(); i++) {

0 commit comments

Comments
 (0)