resource scopes and close actions

[Michael Zucchi]

A simple case is AVFormatContext.

Once you open a file you want to scan the stream list which is stored as 
an AVStream **.  Both need to be segments to use the varhandle api to 
access fields.

typedef struct AVFormatContext {
  ...
     unsigned int nb_streams;
     /**
      * A list of all streams in the file. New streams are created with
      * avformat_new_stream().
      *
      * - demuxing: streams are created by libavformat in 
avformat_open_input().
      *             If AVFMTCTX_NOHEADER is set in ctx_flags, then new 
streams may also
      *             appear in av_read_frame().
      * - muxing: streams are created by the user before 
avformat_write_header().
      *
      * Freed by libavformat in avformat_free_context().
      */
     AVStream **streams;
  ...
};

An example public field of AVStream:

typedef struct AVStream {
  ...
     /**
      * This is the fundamental unit of time (in seconds) in terms
      * of which frame timestamps are represented.
      *
      * decoding: set by libavformat
      * encoding: May be set by the caller before avformat_write_header() to
      *           provide a hint to the muxer about the desired timebase. In
      *           avformat_write_header(), the muxer will overwrite this 
field
      *           with the timebase that will actually be used for the 
timestamps
      *           written into the file (which may or may not be related 
to the
      *           user-provided one, depending on the format).
      */
     AVRational time_base;
...
};

So my current code generator uses a MemorySegment as the sole 'holder' 
for each non-anonymous type, this allows the base constructor just takes 
that and i have factory methods which can create the segment from an 
address.

public class AVFormatContext {
   final MemorySegment segment;
   AVFormatContext(MemorySegment segment) {
     this.segment = segment;
   }
   public AVFormatContext create(MemorySegment segment) {
     return new ...
  }
   public AVFormatContext create(MemoryAddress addr, ResourceScope scope) {
     return create(MemorySegment.ofAddress(addr, layout.byteSize(), scope);
  }

}

Then for the **streams accessor I return a typed List implementation, it 
invokes a BiFunction<MemoryAddress,ResourceScope,R> to create instances.

     public HandleArray<AVStream> getStreams() {
         return 
HandleArray.createArray((MemoryAddress)streams$VH.get(segment()), 
getNumStreams(), AVStream::create, segment.scope());
     }

AVRational is an inline struct so that that also needs it's own segment, 
so again the scope gets passed down to getTimeBase() (implicitly though, 
via a sliceHandle).  So that's that part, it just tracks the same scope 
through the process, but this is more of an issue in relation to the 
other email.  For writing streams you have to create the streams array 
and write entries to it as well, so care would have to be taken to use 
the same scope for everything that is manually created.

The factory for an AVFormatContext takes an explicit scope parameter 
because it's the root 'allocation' if you will, and I was just 
experimenting with adding an auto-close feature to it.

     static final MethodHandle avformat_open_input$FH = 
Memory.downcall("avformat_open_input", FunctionDescriptor.of(Memory.INT, 
Memory.POINTER, Memory.POINTER, Memory.POINTER, Memory.POINTER));
     public static AVFormatContext openInput(String url, AVInputFormat 
fmt, HandleArray<AVDictionary> options, ResourceScope scope$) {
         MemoryAddress ps;
         int result$;
         try (Frame frame$ = Memory.createFrame()) {
             PointerArray ps$h = PointerArray.createArray(1, frame$);
             result$ = 
(int)avformat_open_input$FH.invokeExact((jdk.incubator.foreign.Addressable)Memory.address(ps$h), 
(jdk.incubator.foreign.Addressable)frame$.copy(url), 
(jdk.incubator.foreign.Addressable)Memory.address(fmt), 
(jdk.incubator.foreign.Addressable)Memory.address(options));
             ps = ps$h.get(0);
             if (result$ == 0) {
                 AVFormatContext res$ = AVFormatContext.create(ps, scope$);
                 scope$.addCloseAction(() -> close(ps));
                 return res$;
             }
         } catch (Throwable t) {
             throw new RuntimeException(t);
         }
         throw new RuntimeException("error="+result$);
     }

     public static void close(MemoryAddress s) {...}

So it's only a minor inconvenience - it needs to keep another copy of 
the MemoryAddress (ps) around, and it needs to have a static 'close' 
function that takes a MemoryAddress rather than a static or 
objectfunction that uses 'segment'.  The former is just a wasteful copy 
and the latter pollutes the api a bit.  ffmpeg avformat_close_input 
actually takes an AVFormatContext ** and so can be invoked multiple 
times safely, so another explicit close function would be useful as well 
but that's another matter.

Anyway i'm still trying to work out how to fit this all in, i'm passing 
around scopes because MemorySegment.ofAddress() needs it and now i'm 
trying to work out how to fit them to the lifecycle of objects.   This 
was the first attempt at utilising the facilities for some benefit.

But it seems very difficult to get right and somewhat clumsy to use and 
only provides a very marginal increase in live-ness safety because you 
can always turn a MemoryAddress into a MemorySegment in another scope.  
And ideally it's not something  you'd expose to the api user.  With JNI 
there were other more straightforward mechanisms to manage the java<>c 
lifecycle matching (e.g. nulling out the pointer), ones that could be 
hidden from the library user.

I'm starting to think of changing to use globalScope() and to mostly 
pretend ResourceScope doesn't exist.  For handle-based apis (anonymous 
pointers) like OpenCL it's already effectively the case anyway.


On 2/2/22 21:20, Maurizio Cimadamore wrote:
> Hi Michael,
> I understand the concern. Running close actions _after_ the scope is 
> closed (and so segments associated with it are inaccessible) is a sort 
> of a forced move: you need to run the close actions when you are 100% 
> sure that nobody else is accessing the scope. Otherwise you might have 
> a race between a close action attempting to free some resource and 
> another thread attempting to access it.
>
> The result is that, as you have noticed, adding close actions is 
> mostly useful when working with APIs that turn raw MemoryAddress into 
> segments manually (MemorySegment::ofAddress).
>


> Can you please share a problematic case with ffmpeg?
>
> Thanks
> Maurizio
>
> On 02/02/2022 01:34, Michael Zucchi wrote:
>>
>> Morning,
>>
>> I was hoping to use resource scopes to auto-cleanup native-allocated 
>> resources, but it looks like it's going to be a bit messy because 
>> segments get closed before their close actions are invoked.
>>
>> Is this the intended behaviour or a side-effect of 'no particular 
>> order'?
>>
>> For some api's it doesn't matter as they don't use api-allocated 
>> public structures (e.g. opencl) and the same approach is fine using 
>> MemoryAddress, but for others it's an issue (e.g. ffmpeg) where the 
>> obvious mapping is to use a segment and varhandles.
>>
>> Probably as a workaround - if i want to have this feature at any rate 
>> because i'm not sure if it's a good idea yet - i'll just save a copy 
>> of the address before it's turned into a segment and pass that to any 
>> close function instead.
>>
>> I've only really started looking at seeing how to 'scope' things 
>> 'properly' but it feels a bit clumsy and sort of not worth it because 
>> it has to be worked around all the time either with hacks like this 
>> or resorting to simply using the global scope which effectively does 
>> nothing.
>>
>>  Z
>>
>>