ezrosent / frawk

an efficient awk-like language
Apache License 2.0
1.24k stars 34 forks source link

close(filehandle) is not executed. #102

Closed ghuls closed 11 months ago

ghuls commented 11 months ago

It looks like close(filehandle) constructs are not executed and filehandles are not closed:

# Set soft limit of open files to 30. 
❯  ulimit -S -n 30C

# Frawk runs out of file handles.
❯  frawk 'BEGIN { for (i=0; i<=50; i++) { echo_cmd = sprintf("echo %d", i); while((echo_cmd | getline line) > 0) { print i, line }; close(echo_cmd); } }' 
0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9
10 10
11 11
12 12
13 13
14 14
15 15
16 16
17 17
18 18
19 19
20 20
21 21
22 22
23 23
24 24
25 25
failure in runtime unexpected error when reading error status of file: [src/runtime/mod.rs:410:27] failed to crate command for reading: Too many open files (os error 24). Halting execution

# gawk closes the file handles as expected.
❯  gawk 'BEGIN { for (i=0; i<=50; i++) { echo_cmd = sprintf("echo %d", i); while((echo_cmd | getline line) > 0) { print i, line }; close(echo_cmd); } }' 
0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9
10 10
11 11
12 12
13 13
14 14
15 15
16 16
17 17
18 18
19 19
20 20
21 21
22 22
23 23
24 24
25 25
26 26
27 27
28 28
29 29
30 30
31 31
32 32
33 33
34 34
35 35
36 36
37 37
38 38
39 39
40 40
41 41
42 42
43 43
44 44
45 45
46 46
47 47
48 48
49 49
50 50

# When removing the file handle closing part, gawk also runs out of filehandles:
❯  gawk 'BEGIN { for (i=0; i<=50; i++) { echo_cmd = sprintf("echo %d", i); while((echo_cmd | getline line) > 0) { print i, line };  } }' 
0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9
10 10
11 11
12 12
13 13
14 14
15 15
16 16
17 17
18 18
19 19
20 20
21 21
22 22
23 23
24 24
25 25
gawk: cmd. line:1: fatal: cannot open pipe `echo 26' (Too many open files)

There is also a typo in the error message: Err(e) => err!("failed to crate command for reading: {}", e), to Err(e) => err!("failed to create command for reading: {}", e),

ezrosent commented 11 months ago

NB: the above commit doesn't fix the problem on its own. I'm looking into it, though.

ezrosent commented 11 months ago

I think that last change should fix the issue. Thanks for the easy-to-reproduce failure case!

ghuls commented 11 months ago

Indeed is seems to be solved. Also my original code I had before this reduced example, seems to work now.

Thanks a lot.