Skip to content

Conversation

@IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Oct 24, 2020

The issue at the bottom was submitted to the julia discourse https://discourse.julialang.org/t/segfault-while-loading-images-in-multiple-threads/48878

This PR locks checked_import to ensure that loading is sequential in threaded use cases. The cost of the lock when the module is already loaded is negligible.

checked_import execution times in a png-loading loop:

[ Info: Precompiling ImageIO [82e4d734-157c-48bb-816b-45c225c6df19]
  0.459511 seconds (7.11 k allocations: 546.457 KiB, 4.52% compilation time)
  0.000005 seconds (1 allocation: 16 bytes)
  0.000004 seconds (1 allocation: 16 bytes)
  0.000003 seconds (1 allocation: 16 bytes)

This may slow down parallel loops with heterogenous file types, but even in that case we can't be sure that the IO packages they're loading are themselves using independent deps, so perhaps better to be safe.


Issue from Discourse:

using Images, FileIO
Threads.@threads for path in readdir("img"; join=true)
  load(path)
end
signal (11): Segmentation fault
in expression starting at none:1
in expression starting at none:1
unknown function (ip: 0x7fbd79b0d298)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0e80a)
unknown function (ip: 0x7fbd79b0d294)
unknown function (ip: 0x7fbd79b0d725)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0c8b7)
unknown function (ip: 0x7fbd79b0e80a)
unknown function (ip: 0x7fbd79b0d766)
unknown function (ip: 0x7fbd79b0d725)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0c8b7)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0d766)
unknown function (ip: 0x7fbd79b0e80a)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0d725)
unknown function (ip: 0x7fbd79b0d32d)
unknown function (ip: 0x7fbd79b0d9ec)
unknown function (ip: 0x7fbd79b0e80a)
unknown function (ip: 0x7fbd79b0dafc)
unknown function (ip: 0x7fbd79b0d725)
unknown function (ip: 0x7fbd79b0ea0a)
unknown function (ip: 0x7fbd79b0d9ec)
unknown function (ip: 0x7fbd79b12347)
unknown function (ip: 0x7fbd79b0dafc)
jl_restore_incremental at /usr/bin/../lib/libjulia.so.1 (unknown line)
unknown function (ip: 0x7fbd79b0ea0a)
unknown function (ip: 0x7fbd79b12347)
jl_restore_incremental at /usr/bin/../lib/libjulia.so.1 (unknown line)
_include_from_serialized at ./loading.jl:681
_require_search_from_serialized at ./loading.jl:782
_require_search_from_serialized at ./loading.jl:782
Errors encountered while loading "/path/img/000811.png".
All errors:Errors encountered while loading "/path/img/000001.png".

===========================================Errors encountered while loading "/path/img/001081.png".

All errors:
===========================================
All errors:
===========================================
_require at ./loading.jl:1007
_require at ./loading.jl:1007
require at ./loading.jl:928
require at ./loading.jl:928
require at ./loading.jl:923
require at ./loading.jl:923
unknown function (ip: 0x7fbd79b1cac0)
unknown function (ip: 0x7fbd79b1cac0)
unknown function (ip: 0x7fbd79b1e16e)
unknown function (ip: 0x7fbd79b1e16e)
jl_toplevel_eval_in at /usr/bin/../lib/libjulia.so.1 (unknown line)
jl_toplevel_eval_in at /usr/bin/../lib/libjulia.so.1 (unknown line)
eval at ./boot.jl:331 [inlined]
topimport at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:13
eval at ./boot.jl:331 [inlined]
topimport at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:13
checked_import at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:30
checked_import at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:30
#load#28 at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:195
#load#28 at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:195
load at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:184 [inlined]
#load#14 at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:133 [inlined]
load at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:133 [inlined]
macro expansion at /path/open_images.jl:4 [inlined]
#3#threadsfor_fun at ./threadingconstructs.jl:81
load at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:184 [inlined]
#load#14 at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:133 [inlined]
load at /home/someuser/.julia/packages/FileIO/wN5rD/src/loadsave.jl:133 [inlined]
macro expansion at /path/open_images.jl:4 [inlined]
#3#threadsfor_fun at ./threadingconstructs.jl:81
#3#threadsfor_fun at ./threadingconstructs.jl:48
#3#threadsfor_fun at ./threadingconstructs.jl:48
unknown function (ip: 0x7fbd49e314ac)
unknown function (ip: 0x7fbd49e314ac)
unknown function (ip: 0x7fbd79b053b9)
unknown function (ip: 0x7fbd79b053b9)
unknown function (ip: (nil))
nknown function (ip: (nil))
Allocations: 11229080 (Pool: 11225888; Big: 3192); GC: 8
Allocations: 11229080 (Pool: 11225888; Big: 3192); GC: 8
Segmentation fault (core dumped)

@coveralls
Copy link

coveralls commented Oct 24, 2020

Coverage Status

Coverage decreased (-0.2%) to 73.121% when pulling c292520 on ib/load_lock into 7c2c5a2 on master.

@IanButterworth
Copy link
Member Author

The same fix is needed for ImageIO JuliaIO/ImageIO.jl#11

@codecov
Copy link

codecov bot commented Oct 24, 2020

Codecov Report

Merging #276 into master will decrease coverage by 0.24%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #276      +/-   ##
==========================================
- Coverage   73.36%   73.12%   -0.25%     
==========================================
  Files           8        8              
  Lines         443      439       -4     
==========================================
- Hits          325      321       -4     
  Misses        118      118              
Impacted Files Coverage Δ
src/loadsave.jl 86.66% <90.90%> (-0.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c2c5a2...c292520. Read the comment docs.

@timholy
Copy link
Member

timholy commented Oct 24, 2020

Is there an easy test we can add?

Feel free to merge whenever, though.

@IanButterworth
Copy link
Member Author

I just added a multithreaded test here, but without bringing an external IO dependency into the testing, I don't see an easy way for the module load segfault issue to be fully tested here.

This issue is however tested properly in ImageIO as of JuliaIO/ImageIO.jl#11

I think I'll merge once CI passes (travis seems to be having a bit of a hiccup)

@IanButterworth IanButterworth merged commit 7403247 into master Oct 24, 2020
@IanButterworth IanButterworth deleted the ib/load_lock branch October 24, 2020 16:20
@IanButterworth
Copy link
Member Author

The original user reports that master (as well as ImageIO master) fixes this.

Should I release 1.4.4?

@timholy
Copy link
Member

timholy commented Oct 25, 2020

Yes please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants