Long running C++ NIF segfaults

I’m not an expert in NIFs, but in general the VM yields execution to the NIF when you call it, so you’re really not supposed to have “long-running NIFs”. When you enter a NIF, the VM cannot do GC, context, switching, and so on anymore.

I think a common solution in a case like yours, where you have essentially an external program that needs to run continuously, is to use ports. You spawn the program you want to run and you let your Elixir application manage its lifetime. Then, you communicate with the spawned program through stdout.

Does that make sense?

Hi @whatyouhide, I think I wrongly explained what this NIF does. With “long-running” I mean it has state (internal variables pointing to data structures that can be accessed from the parent Elixir process), please don’t confuse with long loops or heavy processes inside it.

Regarding your ports suggestion, I can do that, but it adds complexity on the C++ side and I’m trying to maintain that part as small as possible.

This is where the program crashes, I call the image function ~100 times (not always the same number of times) then it crashes:

imagen1200×668 73 KB

I can assure you that the NIF hasn’t been unloaded. You would need to do that manually, and even then the VM takes great care not to pull the rug while anything references it.

I think I can see why it crashes though. What’s preventing image/2 from being called concurrently?

Hi @jhogberg, nothing prevents that (in fact, it will be called concurrently at production), but as far as I understand the function createScaledImage_nif is thread safe (it receives the data that should use to work with by param, no global variables).

I added debug messages (std::cout) everywhere in the createScaledImage_nif, and found the app crashes just before entering into that function.

Imagine that you get several concurrent calls to image/2 with the same sopinstanceuid.

Isn’t there then a pretty good chance that they all end up calling createScaledImage_nif, setMinMaxWindow_nif, and getPNG_nif with the same resource?

Also, would you care to share the code for getPNG_nif?

Yes, the program should allow concurrently call createScaledImage_nif, it has a pointer to a resource with the original image, then this function returns a scaled version of it, never overwriting the original.

Here’s the getPNG_nif function:

ERL_NIF_TERM getPNG_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])                                                                                                                                                                 
{                                                                                                                                                                                                                                            
    dcmfileformat_t *ff_res;                                                                                                                                                                                                                 
                                                                                                                                                                                                                                             
    if(!enif_get_resource(env, argv[0], ff_res_t, (void **) &ff_res)) {                                                                                                                                                                      
        return enif_make_badarg(env);                                                                                                                                                                                                        
    };                                                                                                                                                                                                                                       
    DcmDataset *ds = ff_res->_ff->getDataset();                                                                                                                                                                                              
                                                                                                                                                                                                                                             
    std::vector<ui8> out;                                                                                                                                                                                                                    
    writePNGtoMemory(ff_res->_dicomimage, &out, 0);                                                                                                                                                                                          
    ErlNifBinary bin;                                                                                                                                                                                                                        
    enif_alloc_binary(out.size(), &bin);                                                                                                                                                                                                     
    memcpy(bin.data, out.data(), out.size());                                                                                                                                                                                                
    return enif_make_binary(env, &bin);                                                                                                                                                                                                      
    return enif_make_atom(env, "ok");                                                                                                                                                                                                        
}

I commented the block that uses the cached image (lines 27 to 38) and the program didn’t segfaults. It looks like something is filling the cachedDcmFF variable with garbage:

 22   def image(conn, params) do                                                                                                                                                                                                             
 23     sopinstanceuid = params["sopinstanceuid"]                                                                                                                                                                                            
 24     {width, ""} = Integer.parse(params["width"])                                                                                                                                                                                         
 25     {height, ""} = Integer.parse(params["height"])                                                                                                                                                                                       
 26     cachedDcmFF = ImageCache.get(sopinstanceuid)                                                                                                                                                                                         
 27     if cachedDcmFF do                                                                                                                                                                                                                    
 28       IO.puts("====> CACHED! <====")                                                                                                                                                                                                     
 29       Dcmtknif.createScaledImage(cachedDcmFF, width, height, 0, 0);                                                                                                                                                                      
 30       IO.puts("====> createScaledImage <====")                                                                                                                                                                                           
 31       Dcmtknif.setMinMaxWindow(cachedDcmFF)                                                                                                                                                                                              
 32       IO.puts("====> setMinMaxWindow <====")                                                                                                                                                                                             
 33       pngimage = Dcmtknif.getPNG(cachedDcmFF)                                                                                                                                                                                            
 34       IO.puts("====> getPNG <====")                                                                                                                                                                                                      
 35       conn                                                                                                                                                                                                                               
 36       |> put_resp_content_type("image/png")                                                                                                                                                                                              
 37       |> send_resp(200, pngimage)                                                                                                                                                                                                        
 38     else                                                                                                                                                                                                                                 
 39       im = ApiController.image(sopinstanceuid)                                                                                                                                                                                           
 40       IO.puts(im.imagepath)                                                                                                                                                                                                              
 41       if File.exists?(im.imagepath) do                                                                                                                                                                                                   
 42         IO.puts("====> NOT CACHED, LOADING FROM FILE <====")                                                                                                                                                                             
 43         dicomfile = im.imagepath                                                                                                                                                                                                         
 44         dcmff = Dcmtknif.load(dicomfile)                                                                                                                                                                                                 
 45         ImageCache.put(sopinstanceuid, dcmff)                                                                                                                                                                                            
 46         Dcmtknif.createScaledImage(dcmff, width, height, 0, 0);                                                                                                                                                                          
 47         Dcmtknif.setMinMaxWindow(dcmff)                                                                                                                                                                                                  
 48         pngimage = Dcmtknif.getPNG(dcmff)                                                                                                                                                                                                
 49         conn                                                                                                                                                                                                                             
 50         |> put_resp_content_type("image/png")                                                                                                                                                                                            
 51         |> send_resp(200, pngimage)                                                                                                                                                                                                      
 52       else                                                                                                                                                                                                                               
 53         conn                                                                                                                                                                                                                             
 54         |> send_resp(404, "File not found")                                                                                                                                                                                              
 55       end                                                                                                                                                                                                                                
 56     end                                                                                                                                                                                                                                  
 57   end

You are overwriting ff_res->_dicomimage however, so you’ll at the very least be leaking memory.

Please share what happens to ff_res->_dicomimage inside writePNGtoMemory, is there a delete ff_res->_dicomimage or similar in there?

Concurrent accesses would do that.

Maybe this is the culpit:

dcmff = Dcmtknif.load(dicomfile)

Elixir:

def load(dicomfile) do                                                                                                                                                                                                                     
  _loadDicomFile(String.to_charlist(dicomfile))                                                                                                                                                                                            
end

C++

DcmFileFormat * loadDicomFile(const char * dicomfile){                                                                                                                                                                                       
  DcmFileFormat *ff = new DcmFileFormat();                                                                                                                                                                                                   
  if (ff->loadFile(dicomfile).good())                                                                                                                                                                                                        
  {                                                                                                                                                                                                                                          
    return ff;                                                                                                                                                                                                                               
  }                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                             
  return NULL;                                                                                                                                                                                                                               
}

ERL_NIF_TERM loadDicomFile_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])                                                                                                                                                          
{                                                                                                                                                                                                                                            
  char dicomfile[MAXBUFLEN];                                                                                                                                                                                                                 
  enif_get_string(env, argv[0], dicomfile, sizeof(dicomfile), ERL_NIF_LATIN1);                                                                                                                                                               
                                                                                                                                                                                                                                             
  DcmFileFormat *ff = loadDicomFile(dicomfile);                                                                                                                                                                                              
                                                                                                                                                                                                                                             
  // Let's allocate the memory for a DcmFileFormat * pointer                                                                                                                                                                                 
  dcmfileformat_t *ff_res = (dcmfileformat_t *)enif_alloc_resource(ff_res_t, sizeof(dcmfileformat_t));                                                                                                                                       
  if (ff_res == NULL){                                                                                                                                                                                                                       
    std::cout << "ff_res = NULL" << std::endl;                                                                                                                                                                                               
    enif_make_badarg(env);                                                                                                                                                                                                                   
  }                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                             
  ff_res->_ff = ff;                                                                                                                                                                                                                          
                                                                                                                                                                                                                                             
  // We can now make the Erlang term that holds the resource                                                                                                                                                                                 
  ERL_NIF_TERM term = enif_make_resource(env, ff_res);                                                                                                                                                                                       
                                                                                                                                                                                                                                             
  // ...and release the resource so that it will be freed when Erlang garbage collects                                                                                                                                                       
  enif_release_resource(ff_res);                                                                                                                                                                                                             
                                                                                                                                                                                                                                             
  return term;                                                                                                                                                                                                                               
}