Skip to content

Commit a56604a

Browse files
BCSharpBCSharp
authored
Fix file descriptor leak in FileManager (#1711)
* Fix leak in FileManager * Fix more leaks * Update after review
1 parent 5718b37 commit a56604a

8 files changed

Lines changed: 281 additions & 128 deletions

File tree

‎Src/IronPython.Modules/mmap.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public MmapDefault(CodeContext/*!*/ context, int fileno, long length, string tag
147147
_offset = offset;
148148

149149
PythonContext pContext = context.LanguageContext;
150-
if (pContext.FileManager.TryGetFileFromId(pContext, fileno, out PythonIOModule.FileIO fileio)) {
150+
if (pContext.FileManager.TryGetFileFromId(fileno, out PythonIOModule.FileIO fileio)) {
151151
if ((_sourceStream = fileio._readStream as FileStream) == null) {
152152
throw WindowsError(PythonExceptions._OSError.ERROR_INVALID_HANDLE);
153153
}

‎Src/IronPython.Modules/msvcrt.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ to os.fdopen() to create a file object.
6565
public static int open_osfhandle(CodeContext context, BigInteger os_handle, int flags) {
6666
if ((flags & O_TEXT) != 0) throw new NotImplementedException();
6767
FileStream stream = new FileStream(new SafeFileHandle(new IntPtr((long)os_handle), true), FileAccess.ReadWrite);
68-
return context.LanguageContext.FileManager.AddToStrongMapping(stream);
68+
return context.LanguageContext.FileManager.AddStream(stream);
6969
}
7070

7171
private static bool TryGetFileHandle(Stream stream, out object handle) {
@@ -97,7 +97,7 @@ private static bool TryGetFileHandle(Stream stream, out object handle) {
9797
if fd is not recognized.")]
9898
public static object get_osfhandle(CodeContext context, int fd) {
9999
// TODO: handle other FileManager types?
100-
var pfile = context.LanguageContext.FileManager.GetFileFromId(context.LanguageContext, fd);
100+
var pfile = context.LanguageContext.FileManager.GetFileFromId(fd);
101101

102102
object handle;
103103
if (TryGetFileHandle(pfile._readStream, out handle)) return handle;

‎Src/IronPython.Modules/nt.cs‎

Lines changed: 53 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -332,16 +332,14 @@ public static void chmod(CodeContext context, object? path, int mode, [ParamDict
332332
#endif
333333

334334
public static void close(CodeContext/*!*/ context, int fd) {
335-
PythonContext pythonContext = context.LanguageContext;
336-
PythonFileManager fileManager = pythonContext.FileManager;
337-
if (fileManager.TryGetFileFromId(pythonContext, fd, out PythonIOModule.FileIO file)) {
338-
fileManager.CloseIfLast(context, fd, file);
335+
PythonFileManager fileManager = context.LanguageContext.FileManager;
336+
if (fileManager.TryGetFileFromId(fd, out PythonIOModule.FileIO? file)) {
337+
file.closefd = true;
338+
file.close(context);
339339
} else {
340-
Stream? stream = fileManager.GetObjectFromId(fd) as Stream;
341-
if (stream == null) {
342-
throw PythonOps.OSError(9, "Bad file descriptor");
343-
}
344-
fileManager.CloseIfLast(fd, stream);
340+
Stream stream = fileManager.GetStreamFromId(fd);
341+
fileManager.RemoveObjectOnId(fd);
342+
fileManager.DerefAndCloseIfLast(stream);
345343
}
346344
}
347345

@@ -355,65 +353,55 @@ public static void closerange(CodeContext/*!*/ context, int fd_low, int fd_high)
355353
}
356354
}
357355

358-
private static bool IsValidFd(CodeContext/*!*/ context, int fd) {
359-
PythonContext pythonContext = context.LanguageContext;
360-
if (pythonContext.FileManager.TryGetFileFromId(pythonContext, fd, out PythonIOModule.FileIO _)) {
361-
return true;
362-
}
363-
if (pythonContext.FileManager.TryGetObjectFromId(pythonContext, fd, out object o)) {
364-
if (o is Stream) {
365-
return true;
366-
}
367-
}
368-
return false;
369-
}
370-
371356
public static int dup(CodeContext/*!*/ context, int fd) {
372-
PythonContext pythonContext = context.LanguageContext;
373-
if (pythonContext.FileManager.TryGetFileFromId(pythonContext, fd, out PythonIOModule.FileIO file)) {
374-
return pythonContext.FileManager.AddToStrongMapping(file);
357+
PythonFileManager fileManager = context.LanguageContext.FileManager;
358+
359+
object obj = fileManager.GetObjectFromId(fd); // OSError if fd not valid
360+
if (obj is PythonIOModule.FileIO file) {
361+
var file2 = new PythonIOModule.FileIO(context, file.fileno(context));
362+
int fd2 = fileManager.AddFile(file2);
363+
fileManager.EnsureRef(file._readStream);
364+
fileManager.AddRef(file2._readStream);
365+
return fd2;
375366
} else {
376-
Stream? stream = pythonContext.FileManager.GetObjectFromId(fd) as Stream;
377-
if (stream == null) {
378-
throw PythonOps.OSError(9, "Bad file descriptor");
379-
}
380-
return pythonContext.FileManager.AddToStrongMapping(stream);
367+
var stream = (Stream)obj;
368+
fileManager.EnsureRef(stream);
369+
fileManager.AddRef(stream);
370+
return fileManager.AddStream(stream);
381371
}
382372
}
383373

384374

385375
public static int dup2(CodeContext/*!*/ context, int fd, int fd2) {
386-
PythonContext pythonContext = context.LanguageContext;
387-
388-
if (!IsValidFd(context, fd)) {
389-
throw PythonOps.OSError(9, "Bad file descriptor");
390-
}
376+
PythonFileManager fileManager = context.LanguageContext.FileManager;
391377

392-
if (!pythonContext.FileManager.ValidateFdRange(fd2)) {
393-
throw PythonOps.OSError(9, "Bad file descriptor");
378+
object obj = fileManager.GetObjectFromId(fd); // OSError if fd not valid
379+
if (fd == fd2) {
380+
return fd2;
394381
}
395382

396-
bool fd2Valid = IsValidFd(context, fd2);
397-
398-
if (fd == fd2) {
399-
if (fd2Valid) {
400-
return fd2;
401-
}
383+
if (!fileManager.ValidateFdRange(fd2)) {
402384
throw PythonOps.OSError(9, "Bad file descriptor");
403385
}
404386

405-
if (fd2Valid) {
387+
if (fileManager.TryGetObjectFromId(fd2, out _)) {
406388
close(context, fd2);
407389
}
408390

409-
if (pythonContext.FileManager.TryGetFileFromId(pythonContext, fd, out PythonIOModule.FileIO file)) {
410-
return pythonContext.FileManager.AddToStrongMapping(file, fd2);
411-
}
412-
var stream = pythonContext.FileManager.GetObjectFromId(fd) as Stream;
413-
if (stream == null) {
414-
throw PythonOps.OSError(9, "Bad file descriptor");
391+
// TODO: race condition: `open` or `dup` on another thread may occupy fd2
392+
393+
if (obj is PythonIOModule.FileIO file) {
394+
var file2 = new PythonIOModule.FileIO(context, file.fileno(context));
395+
fileManager.AddFile(fd2, file2);
396+
fileManager.EnsureRef(file._readStream);
397+
fileManager.AddRef(file2._readStream);
398+
return fd2;
399+
} else {
400+
var stream = (Stream)obj;
401+
fileManager.EnsureRef(stream);
402+
fileManager.AddRef(stream);
403+
return fileManager.AddStream(fd2, stream);
415404
}
416-
return pythonContext.FileManager.AddToStrongMapping(stream, fd2);
417405
}
418406

419407
#if FEATURE_PROCESS
@@ -436,8 +424,9 @@ public static object fspath(CodeContext context, [AllowNull] object path)
436424

437425
[LightThrowing]
438426
public static object fstat(CodeContext/*!*/ context, int fd) {
439-
PythonContext pythonContext = context.LanguageContext;
440-
pythonContext.FileManager.TryGetObjectFromId(pythonContext, fd, out object obj);
427+
PythonFileManager fileManager = context.LanguageContext.FileManager;
428+
429+
fileManager.TryGetObjectFromId(fd, out object? obj);
441430
if (obj is PythonIOModule.FileIO file) {
442431
if (file.IsConsole) return new stat_result(0x2000);
443432
if (StatStream(file._readStream) is not null and var res) return res;
@@ -463,8 +452,8 @@ static bool IsUnixStream(Stream stream) {
463452
}
464453

465454
public static void fsync(CodeContext context, int fd) {
466-
PythonContext pythonContext = context.LanguageContext;
467-
var pf = pythonContext.FileManager.GetFileFromId(pythonContext, fd);
455+
PythonFileManager fileManager = context.LanguageContext.FileManager;
456+
var pf = fileManager.GetFileFromId(fd);
468457
try {
469458
pf.flush(context);
470459
} catch (Exception ex) when (ex is ValueErrorException || ex is IOException) {
@@ -523,7 +512,7 @@ static void linkUnix(string src, string dst) {
523512
}
524513

525514
public static bool isatty(CodeContext context, int fd) {
526-
if (context.LanguageContext.FileManager.TryGetFileFromId(context.LanguageContext, fd, out var file))
515+
if (context.LanguageContext.FileManager.TryGetFileFromId(fd, out var file))
527516
return file.isatty(context);
528517
return false;
529518
}
@@ -578,8 +567,8 @@ public static PythonList listdir(CodeContext context, [NotNone] Bytes path) {
578567
public static PythonList listdir(CodeContext context, object? path)
579568
=> listdir(context, ConvertToFsString(context, path, nameof(path)));
580569

581-
public static BigInteger lseek(CodeContext context, int filedes, long offset, int whence) {
582-
var file = context.LanguageContext.FileManager.GetFileFromId(context.LanguageContext, filedes);
570+
public static BigInteger lseek(CodeContext context, int fd, long offset, int whence) {
571+
var file = context.LanguageContext.FileManager.GetFileFromId(fd);
583572

584573
return file.seek(context, offset, whence);
585574
}
@@ -867,7 +856,7 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f
867856
fs = new FileStream(path, fileMode, access, FileShare.ReadWrite, DefaultBufferSize, options);
868857
}
869858

870-
return context.LanguageContext.FileManager.AddToStrongMapping(new PythonIOModule.FileIO(context, fs) { name = path });
859+
return context.LanguageContext.FileManager.AddFile(new PythonIOModule.FileIO(context, fs) { name = path, closefd = false });
871860
} catch (Exception e) {
872861
throw ToPythonException(e, path);
873862
}
@@ -924,8 +913,8 @@ public static PythonTuple pipe(CodeContext context) {
924913
var outFile = new PythonIOModule.FileIO(context, pipeStreams.Item2);
925914

926915
return PythonTuple.MakeTuple(
927-
context.LanguageContext.FileManager.AddToStrongMapping(inFile),
928-
context.LanguageContext.FileManager.AddToStrongMapping(outFile)
916+
context.LanguageContext.FileManager.AddFile(inFile),
917+
context.LanguageContext.FileManager.AddFile(outFile)
929918
);
930919
}
931920
#endif
@@ -947,7 +936,7 @@ public static Bytes read(CodeContext/*!*/ context, int fd, int buffersize) {
947936

948937
try {
949938
PythonContext pythonContext = context.LanguageContext;
950-
var pf = pythonContext.FileManager.GetFileFromId(pythonContext, fd);
939+
var pf = pythonContext.FileManager.GetFileFromId(fd);
951940
return (Bytes)pf.read(context, buffersize);
952941
} catch (Exception e) {
953942
throw ToPythonException(e);
@@ -1601,7 +1590,7 @@ public static void truncate(CodeContext context, int fd, BigInteger length)
16011590
=> ftruncate(context, fd, length);
16021591

16031592
public static void ftruncate(CodeContext context, int fd, BigInteger length)
1604-
=> context.LanguageContext.FileManager.GetFileFromId(context.LanguageContext, fd).truncate(context, length);
1593+
=> context.LanguageContext.FileManager.GetFileFromId(fd).truncate(context, length);
16051594

16061595
#if FEATURE_FILESYSTEM
16071596
public static object times() {
@@ -1828,7 +1817,7 @@ public static PythonTuple waitpid(int pid, int options) {
18281817
public static int write(CodeContext/*!*/ context, int fd, [NotNone] IBufferProtocol data) {
18291818
try {
18301819
PythonContext pythonContext = context.LanguageContext;
1831-
var pf = pythonContext.FileManager.GetFileFromId(pythonContext, fd);
1820+
var pf = pythonContext.FileManager.GetFileFromId(fd);
18321821
return (int)pf.write(context, data);
18331822
} catch (Exception e) {
18341823
throw ToPythonException(e);

‎Src/IronPython/Modules/_fileio.cs‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public class FileIO : _RawIOBase, IDisposable, IWeakReferenceable, ICodeFormatta
4141
internal Stream _readStream;
4242
private Stream _writeStream;
4343
private bool _closed, _closefd;
44+
internal int _fd = -1;
4445
private WeakRefTracker _tracker;
4546
private PythonContext _context;
4647
public object name;
@@ -90,6 +91,7 @@ public FileIO(CodeContext/*!*/ context, int fd, string mode = "r", bool closefd
9091
Debug.Fail($"{nameof(fileObject)} is of unexpected type {fileObject.GetType().Name}");
9192
}
9293

94+
_fd = fd;
9395
_closefd = closefd;
9496
}
9597

@@ -147,12 +149,14 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo
147149
throw PythonOps.ValueError("opener returned {0}", fd);
148150
}
149151

150-
if (_context.FileManager.TryGetFileFromId(_context, fd, out FileIO file)) {
152+
if (_context.FileManager.TryGetFileFromId(fd, out FileIO file)) {
151153
_readStream = file._readStream;
152154
_writeStream = file._writeStream;
153-
} else if (_context.FileManager.TryGetObjectFromId(_context, fd, out object fileObj) && fileObj is Stream stream) {
155+
} else if (_context.FileManager.TryGetStreamFromId(fd, out Stream stream)) {
154156
_readStream = stream;
155157
_writeStream = stream;
158+
} else {
159+
throw PythonOps.OSError(9, "Bad file descriptor");
156160
}
157161
} else {
158162
throw PythonOps.TypeError("expected integer from opener");
@@ -273,17 +277,27 @@ public override void close(CodeContext/*!*/ context) {
273277
_closed = true;
274278

275279
if (_closefd) {
276-
if (_readStream != null) {
277-
_readStream.Close();
278-
_readStream.Dispose();
280+
PythonFileManager myManager = _context.RawFileManager;
281+
282+
if (_fd >= 0) {
283+
myManager?.RemoveObjectOnId(_fd);
284+
}
285+
286+
bool readStreamClosed = true;
287+
if (_readStream is not null) {
288+
if (myManager is not null) {
289+
readStreamClosed = myManager.DerefAndCloseIfLast(_readStream);
290+
if (readStreamClosed) {
291+
myManager.Remove(_readStream);
292+
}
293+
} else {
294+
_readStream.Close();
295+
}
279296
}
280-
if (_writeStream != null && !ReferenceEquals(_readStream, _writeStream)) {
297+
if (_writeStream is not null && !ReferenceEquals(_readStream, _writeStream) && readStreamClosed) {
281298
_writeStream.Close();
282-
_writeStream.Dispose();
299+
myManager?.Remove(_writeStream);
283300
}
284-
285-
PythonFileManager myManager = _context.RawFileManager;
286-
myManager?.Remove(this);
287301
}
288302
}
289303

@@ -295,9 +309,8 @@ public override bool closed {
295309
}
296310

297311
public bool closefd {
298-
get {
299-
return _closefd;
300-
}
312+
get => _closefd;
313+
internal set => _closefd = value;
301314
}
302315

303316
[Documentation("fileno() -> int. \"file descriptor\".\n\n"
@@ -306,7 +319,10 @@ public bool closefd {
306319
public override int fileno(CodeContext/*!*/ context) {
307320
_checkClosed();
308321

309-
return _context.FileManager.GetOrAssignIdForFile(this);
322+
if (_fd < 0) {
323+
_fd = _context.FileManager.GetOrAssignId(this);
324+
}
325+
return _fd;
310326
}
311327

312328
[Documentation("Flush write buffers, if applicable.\n\n"

‎Src/IronPython/Runtime/PythonContext.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1873,15 +1873,15 @@ private void SetStandardIO() {
18731873
var stdout = PythonIOModule.CreateConsole(this, io, ConsoleStreamType.Output, "<stdout>", out PythonIOModule.FileIO fstdout);
18741874
var stderr = PythonIOModule.CreateConsole(this, io, ConsoleStreamType.ErrorOutput, "<stderr>", out PythonIOModule.FileIO fstderr);
18751875

1876-
FileManager.AddToStrongMapping(fstdin, 0);
1876+
FileManager.AddFile(0, fstdin);
18771877
SetSystemStateValue("__stdin__", stdin);
18781878
SetSystemStateValue("stdin", stdin);
18791879

1880-
FileManager.AddToStrongMapping(fstdout, 1);
1880+
FileManager.AddFile(1, fstdout);
18811881
SetSystemStateValue("__stdout__", stdout);
18821882
SetSystemStateValue("stdout", stdout);
18831883

1884-
FileManager.AddToStrongMapping(fstderr, 2);
1884+
FileManager.AddFile(2, fstderr);
18851885
SetSystemStateValue("__stderr__", stderr);
18861886
SetSystemStateValue("stderr", stderr);
18871887
}

0 commit comments

Comments
 (0)