Thread safe?

[erikogabrielsson]

Hi,

When using the pyisyntax wrapper for this library I noticed segmentation faults when reading with multiple threads. Not sure if this is a Python problem or if libisyntax is not thread safe?

Can you clarify if the library is thread safe?

[Falcury]

Sorry for the late reply!

I haven't tried the pyisyntax wrapper yet, and I'm not 100% sure about whether it 'should' work in the exact use case in the issue you linked to. But, I suspect it is currently not possible to do it in this way.

At least internally, there is some degree of thread-safety to make sure that tiles can be loaded in parallel (also because to load a single tile, there is usually a dependency on other tiles to be loaded first and there is a lot of complex caching stuff and internal synchronization going on). But, this doesn't necessarily translate to thread safety to the external API.

Maybe @alex-virodov knows a more complete answer for how the reader API is implemented right now?

For the streaming implementation in Slidescape I used a separate thread that collects requests for tiles periodically (each frame), and tries to continually satisfy these requests by pulling from a wishlist and reading as many tiles from that list as possible in a somewhat efficient way. I can imagine we could adjust the implementation to work similar to this, to collect tile requests in parallel and them dispatch them in batches periodically (although that would require some more work to be done).

[jonasteuwen]

@Falcury @erikogabrielsson
The library appears to be thread-safe, when I was integrating it in fastslide. I did strip the library though, so I needed to make this change:

  // CRITICAL FIX: Ensure thread-local memory is initialized
  if (!local_thread_memory) {
    init_thread_memory(0, &global_system_info);
  }

in isyntax_openslide_load_tile_coefficients_ll_or_h. I am not sure if this is because of my fork, or because it was not initialized to begin with (it's also probably not the best fix)

[rvause]

I also ran into this recently when writing bindings for libisyntax. The workaround seemed to be to call this for each thread spawned from the main thread.

const std = @import("std");
const c = @cImport({
    @cInclude("libisyntax.h");
});

extern fn init_thread_memory(logical_thread_index: c_int, system_info: *anyopaque) void;
extern var global_system_info: u8;
extern threadlocal var local_thread_memory: ?*anyopaque;

// ...

pub fn initThreadMemory() void {
    init_thread_memory(0, &global_system_info);
}

pub fn deinitThreadMemory() void {
    if (local_thread_memory) |p| {
        std.c.free(p);
        local_thread_memory = null;
    }
}

// ...
// In thread:

initThreadMemory();
defer deinitThreadMemory();

[Falcury]

I recently reworked the thread pool internals in the Slidescape codebase (I'll sync the codebases to port these over; EDIT: done now). It is possible that these addressed some of the threading-related issues.

@rvause Yes it makes some sense to me that a workaround like this might be needed. Libisyntax has its own thread-pool, and each of the worker threads create their own threadlocal memory buffer/scratch space. However, the main/calling thread is expected to participate in executing jobs and so is required to have initialized its own memory buffer beforehand. This design was OK for a tightly controlled application like Slidescape, but maybe not so much for a library setting, where requiring every calling thread to do this is probably not reasonable. I'll try to rethink this.