Skip to content

adding the same C source file redundantly to the same compilation results in a deadlock #7308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
andrewrk opened this issue Dec 5, 2020 · 1 comment · Fixed by #7394
Closed

adding the same C source file redundantly to the same compilation results in a deadlock #7308

andrewrk opened this issue Dec 5, 2020 · 1 comment · Fixed by #7394
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone
0.7.1

Comments

@andrewrk
Copy link
Member

andrewrk commented Dec 5, 2020

To reproduce:

zig build-lib foo.c foo.c

Expected behavior: Even though it would typically cause symbol collisions, it should be allowed, because it is possible to supply each source file with a different set of preprocessor definitions, which would cause the source files to have different sets of symbols.

Actual behavior: Deadlock in the cache system

#0  0x00007f61a230b047 in flock ()
   from /nix/store/9df65igwjmf2wbw0gbrrgair6piqjgmi-glibc-2.31/lib/libc.so.6
#1  0x0000000000cd0d2f in std.os.flock (fd=132, operation=2) at /home/andy/dev/zig/lib/std/os.zig:4180
#2  0x0000000000b5c698 in std.fs.Dir.createFileZ (self=..., 
    sub_path_c=0x7ffea47e6ac0 "d6e0e2162c95843932835e08e7f71648.txt", flags=...)
    at /home/andy/dev/zig/lib/std/fs.zig:904
#3  0x0000000000b570e0 in std.fs.Dir.createFile (self=..., sub_path=..., flags=...)
    at /home/andy/dev/zig/lib/std/fs.zig:835
#4  0x0000000000b52245 in Cache.Manifest.hit (self=0x7ffea47e9b10)
    at /home/andy/dev/zig/src/Cache.zig:256
#5  0x0000000000c3276d in Compilation.updateCObject (comp=0x9b08278, c_object=0x9b0d540, 
    c_comp_progress_node=0x7ffea47ea048) at /home/andy/dev/zig/src/Compilation.zig:1755
#6  0x0000000000c033d5 in Compilation.performAllTheWork (self=0x9b08278)
    at /home/andy/dev/zig/src/Compilation.zig:1435
#7  0x0000000000bfdb13 in Compilation.update (self=0x9b08278)
    at /home/andy/dev/zig/src/Compilation.zig:1227
#8  0x0000000000bab707 in main.updateModule (gpa=0x8d2ec70 <c_allocator_state>, comp=0x9b08278, 
    zir_out_path=..., hook=...) at /home/andy/dev/zig/src/main.zig:1910
#9  0x0000000000aff68b in main.buildOutputType (gpa=0x8d2ec70 <c_allocator_state>, 
    arena=0x7ffea47ef898, all_args=..., arg_mode=...) at /home/andy/dev/zig/src/main.zig:1772
#10 0x0000000000ad749b in main.mainArgs (gpa=0x8d2ec70 <c_allocator_state>, arena=0x7ffea47ef898, 
    args=...) at /home/andy/dev/zig/src/main.zig:162
#11 0x0000000000ad6e0f in main (argc=649, argv=0x7ffea47efad8) at /home/andy/dev/zig/src/stage1.zig:45
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Dec 5, 2020
@andrewrk andrewrk added this to the 0.7.1 milestone Dec 5, 2020
andrewrk added a commit that referenced this issue Dec 11, 2020
@andrewrk
stage2: store Cache files in a map data structure
cfa0227 
Cache now stores files as an StringArrayListHashMap instead of an
ArrayList. The key is the resolved file path, to avoid adding the same
file multiple times into the cache system when it is depended on
multiple times. This avoids re-hashing file contents that have already
been inserted into the cache.

This was an attempt to fix #7308, but it is solving a different problem.
I do think it is worth considering this improvement regardless. On the
other hand, it might be duplicated effort since the layer of code above
this one probably wants to do its own de-duplication as well.

This implementation would be much nicer with #7391.
@andrewrk andrewrk mentioned this issue Dec 11, 2020
Closed
@andrewrk
Copy link
Member Author

One observation here is that if there were different flags for the different object files, then they would get different cache manifests and thus would not deadlock. The deadlock will only happen when the resulting object files will be identical, and therefore would cause symbol collisions. So I think we can detect this situation and report a friendly error message "foo.c is provided multiple times with the same flags".

andrewrk added a commit that referenced this issue Dec 11, 2020
@andrewrk
stage2: detect redundant C/C++ source files
3c1cda8 
Cache exposes BinDigest.

Compilation gains a set of a BinDigest for every C/C++ source file. We
detect when the same source/flags have already been added and emit a
compile error. This prevents a deadlock in the caching system.

Closes #7308
@andrewrk andrewrk mentioned this issue Dec 11, 2020
Merged
andrewrk added a commit that referenced this issue Dec 11, 2020
@andrewrk
stage2: detect redundant C/C++ source files
64a590a 
Cache exposes BinDigest.

Compilation gains a set of a BinDigest for every C/C++ source file. We
detect when the same source/flags have already been added and emit a
compile error. This prevents a deadlock in the caching system.

Closes #7308
andrewrk added a commit that referenced this issue Dec 11, 2020
@andrewrk
stage2: detect redundant C/C++ source files
fd4c98c 
Cache exposes BinDigest.

Compilation gains a set of a BinDigest for every C/C++ source file. We
detect when the same source/flags have already been added and emit a
compile error. This prevents a deadlock in the caching system.

Closes #7308
aarvay pushed a commit to aarvay/zig that referenced this issue Jan 4, 2021
@andrewrk @aarvay
stage2: detect redundant C/C++ source files
f06a17d 
Cache exposes BinDigest.

Compilation gains a set of a BinDigest for every C/C++ source file. We
detect when the same source/flags have already been added and emit a
compile error. This prevents a deadlock in the caching system.

Closes ziglang#7308
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Milestone
0.7.1
1 participant
@andrewrk