diff --git a/pkg/JuliaInterface/Makefile.in b/pkg/JuliaInterface/Makefile.in index b7b58c726..834c91fc2 100644 --- a/pkg/JuliaInterface/Makefile.in +++ b/pkg/JuliaInterface/Makefile.in @@ -2,7 +2,7 @@ # Makefile rules for the JuliaInterface package # KEXT_NAME = JuliaInterface -KEXT_SOURCES = src/JuliaInterface.c src/calls.c src/convert.c src/sync.c +KEXT_SOURCES = src/JuliaInterface.c src/calls.c src/convert.c KEXT_CFLAGS = KEXT_LDFLAGS = diff --git a/pkg/JuliaInterface/src/JuliaInterface.c b/pkg/JuliaInterface/src/JuliaInterface.c index 147ea5f8b..39cff20b0 100644 --- a/pkg/JuliaInterface/src/JuliaInterface.c +++ b/pkg/JuliaInterface/src/JuliaInterface.c @@ -6,7 +6,6 @@ #include "calls.h" #include "convert.h" -#include "sync.h" #include @@ -36,9 +35,7 @@ void handle_jl_exception(void) jl_value_t * string_object = jl_call1(JULIA_FUNC_take_inplace, JULIA_ERROR_IOBuffer); string_object = jl_call1(JULIA_FUNC_String_constructor, string_object); - BEGIN_GAP_SYNC(); ErrorMayQuit(jl_string_data(string_object), 0, 0); - END_GAP_SYNC(); } static jl_module_t * get_module(const char * name) @@ -49,9 +46,7 @@ static jl_module_t * get_module(const char * name) jl_value_t * module_value = jl_eval_string(name); JULIAINTERFACE_EXCEPTION_HANDLER if (!jl_is_module(module_value)) { - BEGIN_GAP_SYNC(); ErrorQuit("Not a module", 0, 0); - END_GAP_SYNC(); } return (jl_module_t *)module_value; } @@ -64,9 +59,7 @@ Obj Func_JULIAINTERFACE_INTERNAL_INIT(Obj self) JULIA_GAPFFE_type = (jl_datatype_t *)jl_get_global(gap_module, jl_symbol("FFE")); if (!JULIA_GAPFFE_type) { - BEGIN_GAP_SYNC(); ErrorMayQuit("Could not locate the GAP.FFE datatype", 0, 0); - END_GAP_SYNC(); } return NULL; } @@ -98,7 +91,6 @@ int is_gapobj(jl_value_t * v) static Obj Func_NewJuliaCFunc(Obj self, Obj julia_function_ptr, Obj args) { - BEGIN_GAP_SYNC(); if (!IS_JULIA_OBJ(julia_function_ptr)) { ErrorMayQuit("NewJuliaCFunc: must be a Julia object", 0, 0); } @@ -106,7 +98,6 @@ static Obj Func_NewJuliaCFunc(Obj self, Obj julia_function_ptr, Obj args) jl_value_t * func_ptr = GET_JULIA_OBJ(julia_function_ptr); void * ptr = jl_unbox_voidpointer(func_ptr); - END_GAP_SYNC(); return NewJuliaCFunc(ptr, args); } @@ -159,7 +150,6 @@ Obj NewJuliaObj(jl_value_t * v) jl_function_t * get_function_from_obj_or_string(Obj func) { jl_function_t * f = NULL; - BEGIN_GAP_SYNC(); if (IS_JULIA_OBJ(func)) { f = (jl_function_t *)GET_JULIA_OBJ(func); } @@ -173,7 +163,6 @@ jl_function_t * get_function_from_obj_or_string(Obj func) } else ErrorMayQuit("argument is not a julia object or string", 0, 0); - END_GAP_SYNC(); return f; } @@ -195,7 +184,6 @@ static Obj Func_JuliaFunction(Obj self, Obj func) */ static Obj Func_JuliaFunctionByModule(Obj self, Obj funcName, Obj moduleName) { - BEGIN_GAP_SYNC(); RequireStringRep("_JuliaFunctionByModule", funcName); RequireStringRep("_JuliaFunctionByModule", moduleName); @@ -205,7 +193,6 @@ static Obj Func_JuliaFunctionByModule(Obj self, Obj funcName, Obj moduleName) jl_function_t * f = jl_get_function(m, CONST_CSTR_STRING(funcName)); if (f == 0) ErrorMayQuit("Function is not defined in julia", 0, 0); - END_GAP_SYNC(); return NewJuliaFunc(f); } @@ -218,14 +205,12 @@ static Obj FuncIS_JULIA_FUNC(Obj self, Obj obj) // Executes the string in the current julia session. static Obj FuncJuliaEvalString(Obj self, Obj string) { - BEGIN_GAP_SYNC(); RequireStringRep("JuliaEvalString", string); // It suffices to use JULIAINTERFACE_EXCEPTION_HANDLER here, as // jl_eval_string is part of the jlapi, so don't have to be wrapped in // JL_TRY/JL_CATCH. jl_value_t * result = jl_eval_string(CONST_CSTR_STRING(string)); - END_GAP_SYNC(); JULIAINTERFACE_EXCEPTION_HANDLER return gap_julia(result); } @@ -234,13 +219,11 @@ static Obj FuncJuliaEvalString(Obj self, Obj string) // :. static Obj FuncJuliaSymbol(Obj self, Obj name) { - BEGIN_GAP_SYNC(); RequireStringRep("JuliaSymbol", name); // jl_symbol never throws an exception and always returns a valid // result, so no need for extra checks. jl_sym_t * julia_symbol = jl_symbol(CONST_CSTR_STRING(name)); - END_GAP_SYNC(); return NewJuliaObj((jl_value_t *)julia_symbol); } @@ -248,12 +231,10 @@ static Obj FuncJuliaSymbol(Obj self, Obj name) // This function is for debugging purposes. static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val) { - BEGIN_GAP_SYNC(); RequireStringRep("JuliaSetVal", name); jl_value_t * julia_obj = julia_gap(val); jl_sym_t * julia_symbol = jl_symbol(CONST_CSTR_STRING(name)); - END_GAP_SYNC(); jl_set_global(jl_main_module, julia_symbol, julia_obj); return 0; } @@ -262,11 +243,9 @@ static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val) // currently bound to the julia identifier . static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name) { - BEGIN_GAP_SYNC(); RequireStringRep("_JuliaGetGlobalVariable", name); jl_sym_t * symbol = jl_symbol(CONST_CSTR_STRING(name)); - END_GAP_SYNC(); if (!jl_boundp(jl_main_module, symbol)) { return Fail; } @@ -278,7 +257,6 @@ static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name) // currently bound to the julia identifier .. static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module) { - BEGIN_GAP_SYNC(); RequireStringRep("_JuliaGetGlobalVariableByModule", name); jl_module_t * m = 0; @@ -296,7 +274,6 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module) 0, 0); } jl_sym_t * symbol = jl_symbol(CONST_CSTR_STRING(name)); - END_GAP_SYNC(); if (!jl_boundp(m, symbol)) { return Fail; } @@ -309,7 +286,6 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module) // must be a julia object GAP object, and a string. static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name) { - BEGIN_GAP_SYNC(); if (!IS_JULIA_OBJ(super_obj)) { ErrorMayQuit( "JuliaGetFieldOfObject: must be a Julia object", 0, @@ -324,7 +300,6 @@ static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name) // JL_TRY/JL_CATCH. jl_value_t * field_value = jl_get_field(extracted_superobj, CONST_CSTR_STRING(field_name)); - END_GAP_SYNC(); JULIAINTERFACE_EXCEPTION_HANDLER return gap_julia(field_value); } @@ -335,7 +310,6 @@ static Obj FuncSTREAM_CALL(Obj self, Obj stream, Obj func, Obj obj) { syJmp_buf readJmpError; - BEGIN_GAP_SYNC(); if (CALL_1ARGS(IsOutputStream, stream) != True) { ErrorQuit("STREAM_CALL: must be an output stream", 0, 0); @@ -351,7 +325,6 @@ static Obj FuncSTREAM_CALL(Obj self, Obj stream, Obj func, Obj obj) if (!CloseOutput()) { ErrorQuit("STREAM_CALL: cannot close output", 0, 0); } - END_GAP_SYNC(); return 0; } @@ -396,8 +369,6 @@ static Int InitKernel(StructInitInfo * module) jl_options.can_inline = 0; } - InitGapSync(); - // init filters and functions InitHdlrFuncsFromTable(GVarFuncs); diff --git a/pkg/JuliaInterface/src/calls.c b/pkg/JuliaInterface/src/calls.c index 1495c3d01..5dcb25a70 100644 --- a/pkg/JuliaInterface/src/calls.c +++ b/pkg/JuliaInterface/src/calls.c @@ -1,6 +1,5 @@ #include "calls.h" #include "convert.h" -#include "sync.h" #include "JuliaInterface.h" @@ -23,7 +22,6 @@ jl_value_t * call_gap_func(Obj func, jl_value_t * args) size_t len = jl_nfields(args); Obj return_value = NULL; - BEGIN_GAP_SYNC(); if (IS_FUNC(func) && len <= 6) { switch (len) { case 0: @@ -73,7 +71,6 @@ jl_value_t * call_gap_func(Obj func, jl_value_t * args) } return_value = CallFuncList(func, arg_list); } - END_GAP_SYNC(); if (return_value == NULL) { return jl_nothing; } @@ -191,7 +188,6 @@ static Obj DoCallJuliaFuncXArg(Obj func, Obj args) // Obj NewJuliaFunc(jl_function_t * function) { - BEGIN_GAP_SYNC(); Obj name = MakeImmString(jl_symbol_name(jl_gf_name(function))); Obj func = NewFunctionT(T_FUNCTION, sizeof(JuliaFuncBag), name, -1, ArgStringToList("arg"), 0); @@ -217,7 +213,6 @@ Obj NewJuliaFunc(jl_function_t * function) SET_BODY_FUNC(func, body); CHANGED_BAG(body); CHANGED_BAG(func); - END_GAP_SYNC(); return func; } @@ -310,7 +305,6 @@ Obj NewJuliaCFunc(void * function, Obj args) { ObjFunc handler; - BEGIN_GAP_SYNC(); switch (LEN_PLIST(args)) { case 0: handler = DoCallJuliaCFunc0Arg; @@ -345,7 +339,6 @@ Obj NewJuliaCFunc(void * function, Obj args) // store it as a valid julia obj (i.e., void ptr). ((JuliaFuncBag *)ADDR_OBJ(func))->juliaFunc = NewJuliaObj(jl_box_voidpointer(function)); - END_GAP_SYNC(); return func; } diff --git a/pkg/JuliaInterface/src/convert.c b/pkg/JuliaInterface/src/convert.c index db42df125..695792838 100644 --- a/pkg/JuliaInterface/src/convert.c +++ b/pkg/JuliaInterface/src/convert.c @@ -1,7 +1,6 @@ #include "convert.h" #include "calls.h" -#include "sync.h" #include "JuliaInterface.h" // This function is used by GAP.jl @@ -29,7 +28,6 @@ jl_value_t * julia_gap(Obj obj) return jl_false; } jl_value_t * result = (jl_value_t *)obj; - BEGIN_GAP_SYNC(); if (TNUM_OBJ(obj) >= FIRST_EXTERNAL_TNUM && CALL_1ARGS(JULIAINTERFACE_IsJuliaWrapper, obj) == True) { obj = CALL_1ARGS(JULIAINTERFACE_JuliaPointer, obj); @@ -49,7 +47,6 @@ jl_value_t * julia_gap(Obj obj) (Int)TNAM_OBJ(obj), 0); } } - END_GAP_SYNC(); return result; } diff --git a/pkg/JuliaInterface/src/sync.c b/pkg/JuliaInterface/src/sync.c deleted file mode 100644 index e70884fd9..000000000 --- a/pkg/JuliaInterface/src/sync.c +++ /dev/null @@ -1,26 +0,0 @@ -#include "sync.h" -#include -#include - -#ifndef HPCGAP -static pthread_mutex_t GapLock; - -void InitGapSync(void) -{ - pthread_mutexattr_t attr; - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&GapLock, &attr); - pthread_mutexattr_destroy(&attr); -} - -void BeginGapSync(void) -{ - pthread_mutex_lock(&GapLock); -} - -void EndGapSync(void) -{ - pthread_mutex_unlock(&GapLock); -} -#endif diff --git a/pkg/JuliaInterface/src/sync.h b/pkg/JuliaInterface/src/sync.h deleted file mode 100644 index 82d9eba26..000000000 --- a/pkg/JuliaInterface/src/sync.h +++ /dev/null @@ -1,20 +0,0 @@ -#ifndef JULIAINTERFACE_SYNC_H -#define JULIAINTERFACE_SYNC_H - -// #define THREADSAFE_GAP_JL 1 - -#include - -#if defined(HPCGAP) || !defined(THREADSAFE_GAP_JL) -#define BEGIN_GAP_SYNC() ((void)0) -#define END_GAP_SYNC() ((void)0) -#else -void BeginGapSync(void); -void EndGapSync(void); -#define BEGIN_GAP_SYNC() BeginGapSync() -#define END_GAP_SYNC() EndGapSync() -#endif - -void InitGapSync(void); - -#endif diff --git a/src/GAP.jl b/src/GAP.jl index 624163a90..efc5d1bb7 100644 --- a/src/GAP.jl +++ b/src/GAP.jl @@ -19,6 +19,7 @@ if !isfile(deps_jl) Pkg.build("GAP") end include(deps_jl) +include("sync.jl") """ @@ -290,7 +291,7 @@ function initialize(argv::Array{String,1}) end function finalize() - ccall((:GAP_finalize, "libgap"), Cvoid, ()) + @lock ccall((:GAP_finalize, "libgap"), Cvoid, ()) end function run_it() @@ -313,7 +314,7 @@ function run_it() # Do not show the main GAP banner by default. push!(cmdline_options, "-b") end - initialize(cmdline_options) + @lock initialize(cmdline_options) if !show_banner # Leave it to GAP's `LoadPackage` whether package banners are shown. @@ -383,10 +384,10 @@ function prompt() # save the current SIGINT handler # HACK: the hardcoded value for SIG_DFL is not portable, revise this # install GAP's SIGINT handler - old_sigint = ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, C_NULL) + old_sigint = @lock ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, C_NULL) # install GAP's SIGINT handler - ccall(:SyInstallAnswerIntr, Cvoid, ()) + @lock ccall(:SyInstallAnswerIntr, Cvoid, ()) # restore GAP's error output disable_error_handler = true @@ -404,7 +405,7 @@ function prompt() Globals.BreakOnError = false # restore signal handler - ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, old_sigint) + @lock ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, old_sigint) # restore GAP error handler disable_error_handler = false diff --git a/src/ccalls.jl b/src/ccalls.jl index 592c92d2f..77f0c0855 100644 --- a/src/ccalls.jl +++ b/src/ccalls.jl @@ -13,13 +13,13 @@ function _GAP_TO_JULIA(ptr::Ptr{Cvoid}) elseif as_int & 2 == 2 return reinterpret(FFE, ptr) end - return ccall(:julia_gap, Any, (Ptr{Cvoid},), ptr) + return @lock ccall(:julia_gap, Any, (Ptr{Cvoid},), ptr) end # # low-level Julia -> GAP conversion # -_JULIA_TO_GAP(val::Any) = ccall(:gap_julia, Ptr{Cvoid}, (Any,), val) +_JULIA_TO_GAP(val::Any) = @lock ccall(:gap_julia, Ptr{Cvoid}, (Any,), val) #_JULIA_TO_GAP(x::Bool) = x ? gap_true : gap_false _JULIA_TO_GAP(x::FFE) = reinterpret(Ptr{Cvoid}, x) _JULIA_TO_GAP(x::GapObj) = pointer_from_objref(x) @@ -28,12 +28,12 @@ function _JULIA_TO_GAP(x::Int) if x in -1<<60:(1<<60-1) return Ptr{Cvoid}(x << 2 | 1) end - return ccall(:ObjInt_Int, Ptr{Cvoid}, (Int,), x) + return @lock_noexcept ccall(:ObjInt_Int, Ptr{Cvoid}, (Int,), x) end function evalstr_ex(cmd::String) - res = ccall(:GAP_EvalString, GapObj, (Ptr{UInt8},), cmd) + res = @lock ccall(:GAP_EvalString, GapObj, (Ptr{UInt8},), cmd) return res end @@ -73,7 +73,8 @@ end # Retrieve the value of a global GAP variable given its name. This function # returns a raw Ptr value, and should only be called by plumbing code. function _ValueGlobalVariable(name::Union{AbstractString,Symbol}) - return ccall(:GAP_ValueGlobalVariable, Ptr{Cvoid}, (Ptr{UInt8},), name) + return @lock_noexcept ccall(:GAP_ValueGlobalVariable, Ptr{Cvoid}, + (Ptr{UInt8},), name) end function ValueGlobalVariable(name::Union{AbstractString,Symbol}) @@ -83,51 +84,60 @@ end # Test whether the global GAP variable with the given name can be assigned to. function CanAssignGlobalVariable(name::Union{AbstractString,Symbol}) - ccall(:GAP_CanAssignGlobalVariable, Bool, (Ptr{UInt8},), name) + @lock_noexcept ccall(:GAP_CanAssignGlobalVariable, Bool, (Ptr{UInt8},), + name) end # Assign a value to the global GAP variable with the given name. This function # assigns a raw Ptr value, and should only be called by plumbing code. function _AssignGlobalVariable(name::Union{AbstractString,Symbol}, value::Ptr{Cvoid}) - ccall(:GAP_AssignGlobalVariable, Cvoid, (Ptr{UInt8}, Ptr{Cvoid}), name, value) + @lock_noexcept ccall(:GAP_AssignGlobalVariable, Cvoid, (Ptr{UInt8}, + Ptr{Cvoid}), name, value) end # Assign a value to the global GAP variable with the given name. function AssignGlobalVariable(name::Union{AbstractString,Symbol}, value::Any) - if !CanAssignGlobalVariable(name) - error("cannot assing to $name in GAP") + @lock begin + if !CanAssignGlobalVariable(name) + error("cannot assing to $name in GAP") + end + tmp = _JULIA_TO_GAP(value) + _AssignGlobalVariable(name, tmp) end - tmp = _JULIA_TO_GAP(value) - _AssignGlobalVariable(name, tmp) end -MakeString(val::String) = ccall(:GAP_MakeString, GapObj, (Ptr{UInt8},), val) +MakeString(val::String) = + @lock_noexcept ccall(:GAP_MakeString, GapObj, (Ptr{UInt8},), val) function CSTR_STRING(val::GapObj) - char_ptr = ccall(:GAP_CSTR_STRING, Ptr{UInt8}, (Any,), val) + char_ptr = @lock_noexcept ccall(:GAP_CSTR_STRING, Ptr{UInt8}, (Any,), val) return deepcopy(unsafe_string(char_ptr))::String end function CSTR_STRING_AS_ARRAY(val::GapObj)::Array{UInt8,1} - string_len = Int64(ccall(:GAP_LenString, Cuint, (Any,), val)) - char_ptr = ccall(:GAP_CSTR_STRING, Ptr{UInt8}, (Any,), val) + string_len = @lock_noexcept Int64(ccall(:GAP_LenString, Cuint, (Any,), val)) + char_ptr = @lock_noexcept ccall(:GAP_CSTR_STRING, Ptr{UInt8}, (Any,), val) return deepcopy(unsafe_wrap(Array{UInt8,1}, char_ptr, string_len)) end -NewPlist(capacity::Int64) = ccall(:GAP_NewPlist, GapObj, (Int64,), capacity) +NewPlist(capacity::Int64) = + @lock_noexcept ccall(:GAP_NewPlist, GapObj, (Int64,), capacity) NewPrecord(capacity::Int64) = ccall(:GAP_NewPrecord, GapObj, (Int64,), capacity) -NEW_MACFLOAT(x::Float64) = ccall(:NEW_MACFLOAT, GapObj, (Cdouble,), x) -ValueMacFloat(x::GapObj) = ccall(:GAP_ValueMacFloat, Cdouble, (Any,), x) -CharWithValue(x::Cuchar) = ccall(:GAP_CharWithValue, GapObj, (Cuchar,), x) -NewJuliaFunc(x::Function) = ccall(:NewJuliaFunc, GapObj, (Any,), x) +NEW_MACFLOAT(x::Float64) = + @lock_noexcept ccall(:NEW_MACFLOAT, GapObj, (Cdouble,), x) +ValueMacFloat(x::GapObj) = + @lock_noexcept ccall(:GAP_ValueMacFloat, Cdouble, (Any,), x) +CharWithValue(x::Cuchar) = + @lock_noexcept ccall(:GAP_CharWithValue, GapObj, (Cuchar,), x) +NewJuliaFunc(x::Function) = + @lock_noexcept ccall(:NewJuliaFunc, GapObj, (Any,), x) function ElmList(x::GapObj, position) - o = ccall(:GAP_ElmList, Ptr{Cvoid}, (Any, Culong), x, Culong(position)) + o = @lock ccall(:GAP_ElmList, Ptr{Cvoid}, (Any, Culong), x, Culong(position)) return _GAP_TO_JULIA(o) end - """ call_gap_func(func::GapObj, args...; kwargs...) @@ -170,15 +180,17 @@ function call_gap_func(func::GapObj, args...; kwargs...) options = true end try - if TNUM_OBJ(func) == T_FUNCTION && length(args) <= 6 - result = _call_gap_func(func, args...) - else - result = ccall(:call_gap_func, Any, (Any, Any), func, args) - end - if options - Globals.PopOptions() + @lock begin + if TNUM_OBJ(func) == T_FUNCTION && length(args) <= 6 + result = _call_gap_func(func, args...) + else + result = ccall(:call_gap_func, Any, (Any, Any), func, args) + end + if options + Globals.PopOptions() + end + result end - return result catch if options Globals.ResetOptionsStack() @@ -343,11 +355,13 @@ Main const Globals = GlobalsType() function getproperty(::GlobalsType, name::Symbol) - v = _ValueGlobalVariable(name) - if v === C_NULL - error("GAP variable $name not bound") + @lock begin + v = _ValueGlobalVariable(name) + if v === C_NULL + error("GAP variable $name not bound") + end + _GAP_TO_JULIA(v) end - return _GAP_TO_JULIA(v) end function hasproperty(::GlobalsType, name::Symbol) @@ -355,11 +369,13 @@ function hasproperty(::GlobalsType, name::Symbol) end function setproperty!(::GlobalsType, name::Symbol, val::Any) - if !CanAssignGlobalVariable(name) - error("cannot assing to $name in GAP") + @lock begin + if !CanAssignGlobalVariable(name) + error("cannot assing to $name in GAP") + end + tmp = (val === nothing) ? C_NULL : _JULIA_TO_GAP(val) + _AssignGlobalVariable(name, tmp) end - tmp = (val === nothing) ? C_NULL : _JULIA_TO_GAP(val) - _AssignGlobalVariable(name, tmp) end propertynames(::GlobalsType) = gap_to_julia(Vector{Symbol}, Globals.NamesGVars()) diff --git a/src/gap_to_julia.jl b/src/gap_to_julia.jl index 61e784f9f..782d44654 100644 --- a/src/gap_to_julia.jl +++ b/src/gap_to_julia.jl @@ -103,13 +103,13 @@ function gap_to_julia(::Type{BigInt}, x::GapObj) Globals.IsInt(x) || throw(ConversionError(x, BigInt)) ## get size of GAP BigInt (in limbs), multiply ## by 64 to get bits - size_limbs = ccall(:GAP_SizeInt, Cint, (Any,), x) + size_limbs = @lock_noexcept ccall(:GAP_SizeInt, Cint, (Any,), x) size = abs(size_limbs * sizeof(UInt) * 8) ## allocate new GMP new_bigint = Base.GMP.MPZ.realloc2(size) new_bigint.size = size_limbs ## Get limb address ptr - addr = ccall(:GAP_AddrInt, Ptr{UInt}, (Any,), x) + addr = @lock_noexcept ccall(:GAP_AddrInt, Ptr{UInt}, (Any,), x) ## Copy limbs unsafe_copyto!(new_bigint.d, addr, abs(size_limbs)) return new_bigint diff --git a/src/julia_to_gap.jl b/src/julia_to_gap.jl index 342c5494e..f998a29de 100644 --- a/src/julia_to_gap.jl +++ b/src/julia_to_gap.jl @@ -57,7 +57,7 @@ function julia_to_gap(x::BigInt) if x in -1<<60:(1<<60-1) return Int64(x) end - return ccall(:MakeObjInt, GapObj, (Ptr{UInt64}, Cint), x.d, x.size) + return @lock_noexcept ccall(:MakeObjInt, GapObj, (Ptr{UInt64}, Cint), x.d, x.size) end ## Rationals diff --git a/src/sync.jl b/src/sync.jl new file mode 100644 index 000000000..8492d5154 --- /dev/null +++ b/src/sync.jl @@ -0,0 +1,145 @@ +module Sync + + @enum SyncMode mutex pin disabled + + const gap_lock = ReentrantLock() + const pinned_thread = Ref{Int}(1) + + @inline function _lock() + Base._lock(gap_lock) + end + + @inline function _unlock() + Base._unlock(gap_lock) + end + + @inline function check_lock() + @assert gap_lock.locked_by === Base.current_task() + end + + @inline function check_pinned() + @assert Threads.threadid() == pinned_thread[] + end + + # To switch between multi-threaded and single-threaded mode, we + # define functions that install the appropriate handlers for lock() + # etc. + # + # This is necessary because otherwise precompilation would fix + # the mode at whatever state it was during precompilation. Thus, + # initially loading GAP.jl in single-threaded mode would also keep + # synchronization off in multi-threaded mode. + # + # However, Julia tracks function dependencies. If a function changes + # upon which another depends, both are being recompiled. Thus, + # by installing the proper version during __init__(), we force + # selective recompilation of the affected functions as needed. + + function _mode_mutex() + Sync.eval(:(@inline function lock(f::Function) + try + _lock() + f() + finally + _unlock() + end + end)) + Sync.eval(:(@inline function lock_noexcept(f::Function) + _lock() + t = f() + _unlock() + t + end)) + Sync.eval(:(@inline function check_lock(f::Function) + check_lock() + f() + end)) + end + + function _mode_pinned() + Sync.eval(:(@inline function lock(f::Function) + check_pinned() + f() + end)) + Sync.eval(:(@inline function lock_noexcept(f::Function) + check_pinned() + f() + end)) + Sync.eval(:(@inline function check_lock(f::Function) + check_pinned() + f() + end)) + end + + function _mode_nolock() + Sync.eval(:(@inline function lock(f::Function) + f() + end)) + Sync.eval(:(@inline function lock_noexcept(f::Function) + f() + end)) + Sync.eval(:(@inline function check_lock(f::Function) + f() + end)) + end + + # The switch() method allows GAP users to switch at runtime between + # various modes: + # + # 1. Sync.pin pins the GAP interpreter to the current thread. It + # cannot be called from other threads. This is the default mode, + # as it combines best performance with thread safety. + # 2. Sync.mutex serializes all GAP calls via a global mutex. This + # offers thread safety and the ability to call GAP from multiple + # threads, but has worse performance than pinning the GAP interpreter + # to a thread. + # 3. Sync.nolock removes all thread-safety checks. It eliminates all + # overhead, but it is up to the user to ensure that no race + # conditions occur. + + function switch(mode::SyncMode) + stack = stacktrace() + file = stack[1].file + for frame in stack[2:end] + if frame.func in [ :lock, :lock_noexcept ] && frame.file == file + @error "trying to change lock mode within critical region" + end + end + if mode == pin + _mode_pinned() + elseif mode == mutex + _mode_mutex() + else # mode == disabled + _mode_nolock() + end + end + + # Initialization is tricky. __init__() can be called from within the + # first lock() call if the module has already been precompiled. Thus, + # we default to enabling synchronization during precompilation and + # then set the actual lock mode during __init__(). Dropping back from + # synchronization being enabled to being disabled is safe, but not the + # other way round. + + _mode_pinned() + + function __init__() + if Threads.nthreads() == 1 + _mode_nolock() + else + _mode_pinned() + end + end +end + +macro lock(expr) + :( Sync.lock(()->$(esc(expr))) ) +end + +macro lock_noexcept(expr) + :( Sync.lock_noexcept(()->$(esc(expr))) ) +end + +macro check_lock(expr) + :( Sync.check_lock(()->$(esc(expr))) ) +end diff --git a/test/runtests.jl b/test/runtests.jl index e90c1df8d..49748b06a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -10,4 +10,5 @@ include("convert.jl") include("macros.jl") include("packages.jl") include("help.jl") +include("sync.jl") # include("testmanual.jl") # leave this out until doctestfilters can be used here diff --git a/test/sync.jl b/test/sync.jl new file mode 100644 index 000000000..b52659efa --- /dev/null +++ b/test/sync.jl @@ -0,0 +1,36 @@ +function run_timed(script :: String, timeout :: Int) + cmd = Cmd(Base.julia_cmd(); + env = merge(ENV, Dict("JULIA_NUM_THREADS" => "2"))) + push!(cmd.exec, "-e", script) + process = open(cmd) + task = @async begin + wait(process) + end + for t in 1:(timeout*10) + sleep(0.1) + if task.state == :done + return process.exitcode == 0 + end + end + kill(process, 9) + return false +end + +@testset "sync" begin + @test run_timed(""" + using GAP + Threads.@threads for i = 1:20000 + GAP.Globals.SymmetricGroup(7) + end + """, 10) + @test run_timed(""" + using GAP + function test() + perm = @gap "()" + Threads.@threads for i = 1:2000 + perm *= GAP.evalstr("(\$(i), \$(i+1))") + end + end + test() + """, 10) +end