diff --git a/modules/internal/ChapelDynamicLoading.chpl b/modules/internal/ChapelDynamicLoading.chpl index ccaa09eaee13..ff2a470b499a 100644 --- a/modules/internal/ChapelDynamicLoading.chpl +++ b/modules/internal/ChapelDynamicLoading.chpl @@ -51,6 +51,10 @@ module ChapelDynamicLoading { return !configErrorsForDynamicLoading(emitErrors=false); } + iter fanToAll() { + for loc in Locales do yield loc; + } + iter fanToAll(skip: locale) { for loc in Locales { if loc != skip then yield loc; @@ -204,15 +208,15 @@ module ChapelDynamicLoading { return false; } - // A store of of all loaded binaries which lives on LOCALE-0. Each time a - // binary is loaded an entry will be made in the store. The interface to the - // stored info is the system pointer retrieved by 'dlopen'. + var chpl_binaryInfoStore = new owned chpl_BinaryInfoStore(); + + // This class stores all the binaries currently loaded by the program. class chpl_BinaryInfoStore { - // The key is the 'dlopen' pointer returned on LOCALE-0. You are able to - // 'dlopen' a symbol multiple times, so you can get the key as needed. - // TODO: After doing so, do we need to call close to drop the internal - // refcount that is maintained by the OS? + // The key is the 'dlopen' pointer returned on 'this.locale'. You are + // able to 'dlopen' a symbol multiple times, so you can get the key as + // needed. Though you should be good about pairing each 'dlopen' call + // with a 'dlclose' call as the OS usually does internal refcounting. var handleToInfo: chpl_lockGuard(chpl_localMap(c_ptr(void), unmanaged chpl_BinaryInfo?)); @@ -222,10 +226,49 @@ module ChapelDynamicLoading { if slot.val != nil then delete slot.val; } } - } - // This is one per entrypoint binary and lives on LOCALE-0. - var chpl_binaryInfoStore = new owned chpl_BinaryInfoStore(); + // Open a system handle on 'this.locale' and use it to check and see + // if a binary already exists for 'path'. If a binary does exist then + // it will be returned. Otherwise, this procedure returns 'nil' and + // either 'errOnThis' or 'handleOnThis' will be set. + proc checkForEntry(path: string, + out handleOnThis: c_ptr(void), + out errOnThis: owned DynLoadError?) { + var ret: unmanaged chpl_BinaryInfo? = nil; + + on this { + var err; + var handle = localDynLoadOpen(path, err); + + // Make sure that the local pointer is closed (discarding the error). + defer { if handle then localDynLoadClose(handle, err); } + + if err != nil { + errOnThis = err; + + } else if handle == nil { + const msg = 'Failed to open \'' + path + '\' on LOCALE-' + + here.id:string; + errOnThis = new DynLoadError(msg); + } + + const check = err == nil && handle != nil; + + if check then manage this.handleToInfo.read() as m { + if m.get(handle, ret) { + assert(ret != nil); + assert(ret!._systemHandles[this.locale.id] == handle); + + } else { + handleOnThis = handle; + handle = nil; + } + } + } + + return ret; + } + } // This class represents a "wide" binary. It contains the state necessary // to load a symbol from the binary on each locale. @@ -238,14 +281,13 @@ module ChapelDynamicLoading { // This is the path that was used to load the binary. const _path: string; - // This is the set of loaded binary pointers, indexed by locale. It lives - // on LOCALE-0. Since it is a local buffer it can only be accessed there! - var _systemPtrs = new chpl_localBuffer(c_ptr(void), numLocales); + // This is the set of loaded binary pointers, indexed by locale. + var _systemHandles = new chpl_localBuffer(c_ptr(void), numLocales); - // Local pointers for loaded symbols on LOCALE-0. These are used to evict - // entries from the procedure pointer cache when the library is finally - // closed. The value is a pair of "wide index" and the symbol name. - var _procPtrToDataLocale0: chpl_localMap(c_ptr(void), (int, string)); + // Local pointers for loaded symbols on 'this.locale'. These can be used + // to evict entries from the procedure pointer cache when the library is + // finally closed. The value is a pair of (wide-index, symbol). + var _procPtrToDataOnThis: chpl_localMap(c_ptr(void), (int, string)); // A pointer to the parent store is used to coordinate load/unload. const _store: borrowed chpl_BinaryInfoStore; @@ -261,106 +303,111 @@ module ChapelDynamicLoading { proc deinit() { _close(); - if _procPtrToDataLocale0.needsDestroy() { + if _procPtrToDataOnThis.needsDestroy() { import MemMove; - for slot in _procPtrToDataLocale0.slots() do MemMove.destroy(slot); + for slot in _procPtrToDataOnThis.slots() do + MemMove.destroy(slot); } } - // Load a binary given a path. + // Load a binary given a path. The binary will live on the locale where + // 'create()' was called for the first time. If called a subsequent + // time on a different locale, there will be comm as the returned binary + // will be non-local. proc type create(path: string, out err: owned DynLoadError?) { - var ret: unmanaged chpl_BinaryInfo? = nil; + if checkForDynamicLoadingErrors(err) then return nil; + + const store = chpl_binaryInfoStore.borrow(); + var handleOnStoreLocale: c_ptr(void); + + if const ret = store.checkForEntry(path, handleOnStoreLocale, err) { + // There was an existing entry, so return it. + return ret; + } else if err != nil { + // There was an error, so return nothing. + return nil; + } - if checkForDynamicLoadingErrors(err) then return ret; + var bin = new owned chpl_BinaryInfo(path, chpl_binaryInfoStore); + var errBuf = new chpl_localBuffer(owned DynLoadError?, numLocales); + var numErrs: atomic int; - on Locales[0] { - const store = chpl_binaryInfoStore.borrow(); - var shouldCreateNewEntry = true; + // No entry and no error, so the handle used should not be 'nil'. + assert(handleOnStoreLocale != nil); + bin._systemHandles[store.locale.id] = handleOnStoreLocale; - // Start by opening a system handle on LOCALE-0. - var err0: owned DynLoadError?; - var ptr0 = localDynLoadOpen(path, err0); + if shouldFanOut { + const skip = store.locale; + + coforall loc in fanToAll(skip=skip) with (ref errBuf) do on loc { + var err; + var ptr = localDynLoadOpen(path, err); - // TODO: Need to propagate/combine this error instead. - defer { if ptr0 then localDynLoadClose(ptr0, err0); } + // Ensure that the local pointer is closed (discarding the error). + defer { if ptr then localDynLoadClose(ptr, err); } + + if ptr { + // Swap in pointer on 'bin'. This clears the variable. + const idx = here.id; + on bin do bin._systemHandles[idx] <=> ptr; + assert(ptr == nil); + + } else if err == nil { + // TODO: Make it so all failure paths return an error. + const msg = 'Failed to open \'' + path + '\' on LOCALE-' + + here.id:string; + err = new DynLoadError(msg); + } - if err0 != nil || ptr0 == nil { - if err0 != nil then err = err0; - shouldCreateNewEntry = false; + if err != nil { + const idx = here.id; + on errBuf do errBuf[idx] = err; + numErrs.add(1); + } } + } - // Check the LOCALE-0 store for an existing entry. - if shouldCreateNewEntry { - manage store.handleToInfo.read() as m { - shouldCreateNewEntry = !m.get(ptr0, ret); + if numErrs.read() > 0 { + // If there were errors, then report the first error. + for i in 0.. ptr0; - - var errBuf = new chpl_localBuffer(owned DynLoadError?, numLocales); - var numErrs: atomic int = 0; - - // Loop and attempt to open system handles on other locales. - if shouldFanOut { - coforall loc in fanToAll(skip=here) with (ref errBuf) do on loc { - const n = (here.id: int); - - var err: owned DynLoadError?; - var ptr = localDynLoadOpen(path, err); - - // TODO: Need to propagate/combine this error instead. - defer { if ptr then localDynLoadClose(ptr, err); } - - if err { - // The buffer is not wide! Assignment occurs on LOCALE-0. - on Locales[0] do errBuf[n] = err; - numErrs.add(1); - } else if ptr { - // Swap in pointer on LOCALE-0. This clears the variable. - on Locales[0] do bin._systemPtrs[n] <=> ptr; - - ptr = nil; - } else { - // TODO: Construct an error instead. - halt('Failed to load binary \'' + path + '\' on ' + - here.id:string + '!'); - } - } + coforall loc in fanToAll() do on loc { + // Clean up all the system handles that have been opened. + const ptr = bin._systemHandles[here.id]; + + if ptr != nil { + var err; + localDynLoadClose(ptr, err); + if err then halt('Error when closing system handle!'); } + } - if numErrs.read() > 0 { - // If there were errors, then report the first error. - // TODO: Consolidate the reported errors. - for i in 0.. 1 { - coforall loc in Locales[1..] do on loc { - local do manage chpl_localPtrCache.guard.write() as m { - const ptr = lookupPtrFromLocalFtable(idx); - m.add(ptr, ret); - } - } + if shouldFanOut && needToEmplaceIdx { + coforall loc in fanToAll(skip=origin) do on loc do local { + manage chpl_localPtrCache.guard.write() as m { + const ptr = lookupPtrFromLocalFtable(idx); + m.add(ptr, ret); } } } diff --git a/test/dynamic-loading/LoadForeignLibrary.chpl b/test/dynamic-loading/LoadForeignLibrary.chpl index 76ba9f29ba77..8c806493ed9c 100644 --- a/test/dynamic-loading/LoadForeignLibrary.chpl +++ b/test/dynamic-loading/LoadForeignLibrary.chpl @@ -230,10 +230,41 @@ proc test4() { } } +// This test demonstrates that a library will persist on the locale that +// it is originally loaded, even if other locales attempt to load it after. +proc test5() { + writeln(getRoutineName()); + + // First create the library on LOCALE-0. This is where it will live. + var bin0 = new dynamicLibrary("./TestCLibraryToLoad.so"); + type P1 = proc(_: int, _: int): int; + const wideProc0 = try! bin0.load("addTwoReturn", P1); + assert(wideProc0 != nil); + + // TODO: Need a way to force evict libraries before execution ends. + assert(bin0._bin!.locale == here); + + coforall loc in Locales do on loc { + var binHere = new dynamicLibrary("./TestCLibraryToLoad.so"); + + // The locales should match. + assert(binHere._bin!.locale == bin0._bin!.locale); + + // And loading should work without incident. + type P1 = proc(_: int, _: int): int; + const wideProcHere = try! binHere.load("addTwoReturn", P1); + assert(wideProcHere != nil); + + // And should have the same wide index. + assert(wideProcHere == wideProc0); + } +} + proc main() { test0(); test1(); test2(); test3(); test4(); + test5(); } diff --git a/test/dynamic-loading/LoadForeignLibrary.comm-none.good b/test/dynamic-loading/LoadForeignLibrary.comm-none.good index 7e32415c00fc..353d273d82df 100644 --- a/test/dynamic-loading/LoadForeignLibrary.comm-none.good +++ b/test/dynamic-loading/LoadForeignLibrary.comm-none.good @@ -18,3 +18,4 @@ Caught error! test3 Caught error! test4 +test5 diff --git a/test/dynamic-loading/LoadForeignLibrary.good b/test/dynamic-loading/LoadForeignLibrary.good index 64708cccbe70..b8db99152bdc 100644 --- a/test/dynamic-loading/LoadForeignLibrary.good +++ b/test/dynamic-loading/LoadForeignLibrary.good @@ -39,3 +39,4 @@ Caught error! test3 Caught error! test4 +test5