add try/catch to all callback functions (never unwind through them) (#206)

Author: StefanKarpinski

src/Curl/Easy.jl

@@ -354,11 +354,16 @@ function header_callback(
     count  :: Csize_t,
     easy_p :: Ptr{Cvoid},
 )::Csize_t
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    n = size * count
-    hdr = unsafe_string(data, n)
-    push!(easy.res_hdrs, hdr)
-    return n
+    try
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        n = size * count
+        hdr = unsafe_string(data, n)
+        push!(easy.res_hdrs, hdr)
+        return n
+    catch err
+        @async @error("header_callback: unexpected error", err=err, maxlog=1_000)
+        return typemax(Csize_t)
+    end
 end
 
 # feed data to read_callback
@@ -380,39 +385,49 @@ function read_callback(
     count  :: Csize_t,
     easy_p :: Ptr{Cvoid},
 )::Csize_t
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    buf = easy.input
-    if buf === nothing
-        notify(easy.ready)
-        return 0 # done uploading
-    end
-    if isempty(buf)
-        notify(easy.ready)
-        return CURL_READFUNC_PAUSE # wait for more data
+    try
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        buf = easy.input
+        if buf === nothing
+            notify(easy.ready)
+            return 0 # done uploading
+        end
+        if isempty(buf)
+            notify(easy.ready)
+            return CURL_READFUNC_PAUSE # wait for more data
+        end
+        n = min(size * count, length(buf))
+        ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), data, buf, n)
+        deleteat!(buf, 1:n)
+        return n
+    catch err
+        @async @error("read_callback: unexpected error", err=err, maxlog=1_000)
+        return CURL_READFUNC_ABORT
     end
-    n = min(size * count, length(buf))
-    ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), data, buf, n)
-    deleteat!(buf, 1:n)
-    return n
 end
 
 function seek_callback(
     easy_p :: Ptr{Cvoid},
     offset :: curl_off_t,
     origin :: Cint,
 )::Cint
-    if origin != 0
-        @async @error("seek_callback: unsupported seek origin", origin, maxlog=1_000)
-        return CURL_SEEKFUNC_CANTSEEK
-    end
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    easy.seeker === nothing && return CURL_SEEKFUNC_CANTSEEK
-    try easy.seeker(offset)
+    try
+        if origin != 0
+            @async @error("seek_callback: unsupported seek origin", origin, maxlog=1_000)
+            return CURL_SEEKFUNC_CANTSEEK
+        end
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        easy.seeker === nothing && return CURL_SEEKFUNC_CANTSEEK
+        try easy.seeker(offset)
+        catch err
+            @async @error("seek_callback: seeker failed", err, maxlog=1_000)
+            return CURL_SEEKFUNC_FAIL
+        end
+        return CURL_SEEKFUNC_OK
     catch err
-        @async @error("seek_callback: seeker failed", err, maxlog=1_000)
+        @async @error("seek_callback: unexpected error", err=err, maxlog=1_000)
         return CURL_SEEKFUNC_FAIL
     end
-    return CURL_SEEKFUNC_OK
 end
 
 function write_callback(
@@ -421,12 +436,17 @@ function write_callback(
     count  :: Csize_t,
     easy_p :: Ptr{Cvoid},
 )::Csize_t
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    n = size * count
-    buf = Array{UInt8}(undef, n)
-    ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), buf, data, n)
-    put!(easy.output, buf)
-    return n
+    try
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        n = size * count
+        buf = Array{UInt8}(undef, n)
+        ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), buf, data, n)
+        put!(easy.output, buf)
+        return n
+    catch err
+        @async @error("write_callback: unexpected error", err=err, maxlog=1_000)
+        return typemax(Csize_t)
+    end
 end
 
 function progress_callback(
@@ -436,9 +456,14 @@ function progress_callback(
     ul_total :: curl_off_t,
     ul_now   :: curl_off_t,
 )::Cint
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    put!(easy.progress, (dl_total, dl_now, ul_total, ul_now))
-    return 0
+    try
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        put!(easy.progress, (dl_total, dl_now, ul_total, ul_now))
+        return 0
+    catch err
+        @async @error("progress_callback: unexpected error", err=err, maxlog=1_000)
+        return -1
+    end
 end
 
 function debug_callback(
@@ -448,10 +473,15 @@ function debug_callback(
     size   :: Csize_t,
     easy_p :: Ptr{Cvoid},
 )::Cint
-    easy = unsafe_pointer_to_objref(easy_p)::Easy
-    @assert easy.handle == handle
-    easy.debug(info_type(type), unsafe_string(data, size))
-    return 0
+    try
+        easy = unsafe_pointer_to_objref(easy_p)::Easy
+        @assert easy.handle == handle
+        easy.debug(info_type(type), unsafe_string(data, size))
+        return 0
+    catch err
+        @async @error("debug_callback: unexpected error", err=err, maxlog=1_000)
+        return -1
+    end
 end
 
 function add_callbacks(easy::Easy)

src/Curl/Multi.jl

@@ -122,22 +122,27 @@ function timer_callback(
     timeout_ms :: Clong,
     multi_p    :: Ptr{Cvoid},
 )::Cint
-    multi = unsafe_pointer_to_objref(multi_p)::Multi
-    @assert multi_h == multi.handle
-    stoptimer!(multi)
-    if timeout_ms >= 0
-        multi.timer = Timer(timeout_ms/1000) do timer
-            lock(multi.lock) do
-                multi.timer === timer || return
-                multi.timer = nothing
-                do_multi(multi)
+    try
+        multi = unsafe_pointer_to_objref(multi_p)::Multi
+        @assert multi_h == multi.handle
+        stoptimer!(multi)
+        if timeout_ms >= 0
+            multi.timer = Timer(timeout_ms/1000) do timer
+                lock(multi.lock) do
+                    multi.timer === timer || return
+                    multi.timer = nothing
+                    do_multi(multi)
+                end
             end
+        elseif timeout_ms != -1
+            @async @error("timer_callback: invalid timeout value", timeout_ms, maxlog=1_000)
+            return -1
         end
-    elseif timeout_ms != -1
-        @async @error("timer_callback: invalid timeout value", timeout_ms, maxlog=1_000)
+        return 0
+    catch err
+        @async @error("timer_callback: unexpected error", err=err, maxlog=1_000)
         return -1
     end
-    return 0
 end
 
 function socket_callback(
@@ -147,44 +152,49 @@ function socket_callback(
     multi_p   :: Ptr{Cvoid},
     watcher_p :: Ptr{Cvoid},
 )::Cint
-    if action ∉ (CURL_POLL_IN, CURL_POLL_OUT, CURL_POLL_INOUT, CURL_POLL_REMOVE)
-        @async @error("socket_callback: unexpected action", action, maxlog=1_000)
-        return -1
-    end
-    multi = unsafe_pointer_to_objref(multi_p)::Multi
-    if watcher_p != C_NULL
-        old_watcher = unsafe_pointer_to_objref(watcher_p)::FDWatcher
-        @check curl_multi_assign(multi.handle, sock, C_NULL)
-        unpreserve_handle(old_watcher)
-    end
-    if action in (CURL_POLL_IN, CURL_POLL_OUT, CURL_POLL_INOUT)
-        readable = action in (CURL_POLL_IN,  CURL_POLL_INOUT)
-        writable = action in (CURL_POLL_OUT, CURL_POLL_INOUT)
-        watcher = FDWatcher(OS_HANDLE(sock), readable, writable)
-        preserve_handle(watcher)
-        watcher_p = pointer_from_objref(watcher)
-        @check curl_multi_assign(multi.handle, sock, watcher_p)
-        task = @async while watcher.readable || watcher.writable # isopen(watcher)
-            events = try
-                wait(watcher)
-            catch err
-                err isa EOFError && return
-                err isa Base.IOError || rethrow()
-                FileWatching.FDEvent()
-            end
-            flags = CURL_CSELECT_IN  * isreadable(events) +
-                    CURL_CSELECT_OUT * iswritable(events) +
-                    CURL_CSELECT_ERR * (events.disconnect || events.timedout)
-            lock(multi.lock) do
-                watcher.readable || watcher.writable || return # !isopen
-                @check curl_multi_socket_action(multi.handle, sock, flags)
-                check_multi_info(multi)
+    try
+        if action ∉ (CURL_POLL_IN, CURL_POLL_OUT, CURL_POLL_INOUT, CURL_POLL_REMOVE)
+            @async @error("socket_callback: unexpected action", action, maxlog=1_000)
+            return -1
+        end
+        multi = unsafe_pointer_to_objref(multi_p)::Multi
+        if watcher_p != C_NULL
+            old_watcher = unsafe_pointer_to_objref(watcher_p)::FDWatcher
+            @check curl_multi_assign(multi.handle, sock, C_NULL)
+            unpreserve_handle(old_watcher)
+        end
+        if action in (CURL_POLL_IN, CURL_POLL_OUT, CURL_POLL_INOUT)
+            readable = action in (CURL_POLL_IN,  CURL_POLL_INOUT)
+            writable = action in (CURL_POLL_OUT, CURL_POLL_INOUT)
+            watcher = FDWatcher(OS_HANDLE(sock), readable, writable)
+            preserve_handle(watcher)
+            watcher_p = pointer_from_objref(watcher)
+            @check curl_multi_assign(multi.handle, sock, watcher_p)
+            task = @async while watcher.readable || watcher.writable # isopen(watcher)
+                events = try
+                    wait(watcher)
+                catch err
+                    err isa EOFError && return
+                    err isa Base.IOError || rethrow()
+                    FileWatching.FDEvent()
+                end
+                flags = CURL_CSELECT_IN  * isreadable(events) +
+                        CURL_CSELECT_OUT * iswritable(events) +
+                        CURL_CSELECT_ERR * (events.disconnect || events.timedout)
+                lock(multi.lock) do
+                    watcher.readable || watcher.writable || return # !isopen
+                    @check curl_multi_socket_action(multi.handle, sock, flags)
+                    check_multi_info(multi)
+                end
             end
+            @isdefined(errormonitor) && errormonitor(task)
         end
-        @isdefined(errormonitor) && errormonitor(task)
+        @isdefined(old_watcher) && close(old_watcher)
+        return 0
+    catch
+        @async @error("socket_callback: unexpected error", err=err, maxlog=1_000)
+        return -1
     end
-    @isdefined(old_watcher) && close(old_watcher)
-    return 0
 end
 
 function add_callbacks(multi::Multi)

test/runtests.jl

@@ -201,11 +201,11 @@ include("setup.jl")
         end
 
         @testset "delete default header" begin
-        headers = [
-            "Accept"     => nothing
-            "User-Agent" => nothing
-        ]
-        json = download_json(url, headers = headers)
+            headers = [
+                "Accept"     => nothing
+                "User-Agent" => nothing
+            ]
+            json = download_json(url, headers = headers)
             @test !("Accept" in keys(json["headers"]))
             @test !("User-Agent" in keys(json["headers"]))
         end