-
Notifications
You must be signed in to change notification settings - Fork 32
Description
Went through mkconcore.py and the file handle situation is pretty bad. Out of 56 open() calls in the file, only about 8 actually use with statements (the .iport/.oport writes near line 579). Everything else is raw open() with manual .close() scattered hundreds of lines later.
Here's what I found:
Config reads never get closed (lines 107, 110, 113)
MCRPATH = open(CONCOREPATH+"/concore.mcr", "r").readline().strip()
DOCKEREXE = open(CONCOREPATH+"/concore.sudo", "r").readline().strip()
DOCKEREPO = open(CONCOREPATH+"/concore.repo", "r").readline().strip()
These just dangle. The file object goes out of scope after .strip() but python doesn't guarantee when GC will actually close it.
GraphML file read (line 182): never closed
f = open(GRAPHML_FILE, "r")
text_str = f.read()
No f.close() anywhere after this. If BeautifulSoup throws on bad XML, handle leaks. Even if it doesn't, it stays open for the rest of the script.
12 template file copies (lines 413–527) : close only on happy path
Same pattern repeated 12 times for concore.py, concore.hpp, concore.v, mkcompile, all the .m files, import_concore.m:
try:
fsource = open(CONCOREPATH+"/concore.py")
except:
logging.error(...)
quit()
with open(outdir+"/src/concore.py","w") as fcopy:
fcopy.write(fsource.read())
fsource.close()
The destination file gets a with block but the source file doesn't. If .read() or .write() throws between open() and fsource.close(), the source handle leaks.
8 script files held open for 900+ lines (lines 150–167, closed at 1115–1122)
fbuild, frun, fdebug, fstop, fclear, fmaxtime, funlock, fparams — all opened at the top and closed at the very bottom. That's 950+ lines of logic where any unhandled exception means all 8 leak. On a system with strict ulimit or if mkconcore gets called in a loop, you'll hit "too many open files".
Fix:
-
Wrap config reads in with
-
Wrap GraphML read in with
-
Wrap all 12 template copies in proper with blocks for both source and destination
-
Restructure the 8 script file writes so they use context managers or at least have try/finally cleanup
Note: I know #122 exists and covers part of this, but that issue only showed one example from the template copies section. The actual scope is way bigger, config reads, the GraphML handle, and the 8 long-lived script handles are all affected too. This issue documents the full picture so the PR can cover everything in one pass.