Thread safety check of MemorySegment

Wed Jul 22 15:50:29 UTC 2020

One thing so, is that the (only) write-transaction per resource internally
might use a scheduler job for auto-committing after a given time, depending
on the constructor-parameters. Internally the I/O reader/writer after a
commit is closed, thus the segment(s) are also closed. so at least here I
assume I can give the ownership from the "main" owning thread to the
scheduler thread :-)

Am Mi., 22. Juli 2020 um 13:33 Uhr schrieb Johannes Lichtenberger <
lichtenberger.johannes at gmail.com>:

> On 22/07/2020 12:07, Johannes Lichtenberger wrote:
>>
>> I just had a quick look at the code, but I think I'm wrong. If
>> transactions are not closed by it's transaction instance handle directly
>> they are closed by the resource manager (because it keeps track of the
>> started transactions) and the resource manager often times is opened on the
>> main event loop (that might not be needed always, but still). I think in
>> the long-run it really complicates stuff and we have to be super careful.
>> It might even be impossible to know in advance on which thread a coroutine
>> will be executed.
>>
>> So... you have a thread which creates (and accesses the segment).
>>
>> But then you have another thread which can close the segment.
>>
>> How does your logic make sure that the second thread cannot close the
>> segment while the first is still operating on it? Is there a point in the
>> code where the original thread hand over control to the second thread (the
>> transaction one) ?
>>
> Basically it's not checked, just documented in the JavaDoc, that only one
> thread at a time should handle the transaction. I think it's at least bad
> style and I should refactor this.
>
> So, for instance a time travel query might internally open a bunch of
> transactions, as a transaction is always bound to one revision, thus it can
> only access this version of the persistent, durable tree. The resource
> manager however is sometimes opened in the other thread. The serializer,
> which materializes the nodes / items as of now doesn't close the
> transactions itself, thus the resource manager from the other thread has to
> close the transactions. That might be not necessary.
>
> I think in my case through careful thinking and refactoring I can get rid
> of this, such that the database + resource manager + transactions are
> always opened and closed in the same thread. I think code reviews would be
> the best (also regarding clean code principles), but currently I'm lacking
> an Open Source community as I didn't release a major version, that is
> 1.0.0. Well well, we'll see :-)
>
> kind regards
> Johannes
>
> Am Mi., 22. Juli 2020 um 13:10 Uhr schrieb Maurizio Cimadamore <
> maurizio.cimadamore at oracle.com>:
>
>>
>> On 22/07/2020 12:07, Johannes Lichtenberger wrote:
>>
>> I just had a quick look at the code, but I think I'm wrong. If
>> transactions are not closed by it's transaction instance handle directly
>> they are closed by the resource manager (because it keeps track of the
>> started transactions) and the resource manager often times is opened on the
>> main event loop (that might not be needed always, but still). I think in
>> the long-run it really complicates stuff and we have to be super careful.
>> It might even be impossible to know in advance on which thread a coroutine
>> will be executed.
>>
>> So... you have a thread which creates (and accesses the segment).
>>
>> But then you have another thread which can close the segment.
>>
>> How does your logic make sure that the second thread cannot close the
>> segment while the first is still operating on it? Is there a point in the
>> code where the original thread hand over control to the second thread (the
>> transaction one) ?
>>
>> Maurizio
>>
>>
>> kind regards
>> Johannes
>>
>> Am Mi., 22. Juli 2020 um 12:58 Uhr schrieb Johannes Lichtenberger <
>> lichtenberger.johannes at gmail.com>:
>>
>>> Transfer of ownership must be initiated by the thread that owns the
>>>> segment. If that works for you, then, yes, it can be used as a solution.
>>>>
>>>
>>> I think in my case the code is not so much. I'm using Vert.x with
>>> Kotlin/Coroutines for a REST-API and that currently is where the problem
>>> stems from. I think I can even in most cases open the transaction in the
>>> "worker" thread instead of the main loop. Thus, I guess it's doable without
>>> too much work. In my case I might also be lucky, that I basically as of now
>>> have no external projects, which embed SirixDB. In any case the read-write
>>> transaction started in SirixDB also should be only used by one thread at a
>>> time, but I'm simply not checking if it runs on another thread when closing.
>>>
>>> Tracking down the issue in my case was a bit annoying, because the
>>> exception was caused in the close-method of the transaction and then the
>>> transaction didn't close properly, but the exception wasn't thrown --
>>> there's clearly an issue somehow with the exception handling in my REST-API
>>> as it simply should be returned from the server :-/ Just during a resource
>>> deletion, an exception popped up, that open resource managers (which start
>>> transactions) were found.
>>>
>>> kind regards
>>> Johannes
>>>
>>> Am Mi., 22. Juli 2020 um 12:43 Uhr schrieb Maurizio Cimadamore <
>>> maurizio.cimadamore at oracle.com>:
>>>
>>>>
>>>> On 22/07/2020 11:26, Johannes Lichtenberger wrote:
>>>>
>>>> I'm also using a mapped memory segment. But I think at least we have
>>>> the possibility to transfer ownership in Java 15, right? Thus, in my
>>>> AutoClosable in the close() method I can simply transfer ownership to the
>>>> current thread if the segment is still alive :-)
>>>>
>>>> Transfer of ownership must be initiated by the thread that owns the
>>>> segment. If that works for you, then, yes, it can be used as a solution.
>>>>
>>>> I was chatting with Jorn earlier, and he was thinking of something that
>>>> I tried to implement at some point, but then I gave up; basically, the big
>>>> issues with withOwnerThread is that (1) must be called by owner thread and
>>>> (2) the owner thread must know the thread picking it up.
>>>>
>>>> It is in principle possible to break up withOwnerThread in two halves:
>>>> e.g. detach (removes ownership) +  attach (re-adds ownership to a detached
>>>> segment).
>>>>
>>>> So, if in your use cases, the workflow is clear enough that you know
>>>> _when_ the segment is no longer being worked on, the owner thread could
>>>> simply call something like MemorySegment::detach (and maybe put the segment
>>>> on a queue); later on, another thread (a cleaner one?) might pick it up,
>>>> attach to it, and call close.
>>>>
>>>> Is this something that would help in the use cases being considered in
>>>> this thread? Or does it still fall in the "too restrictive" category? My
>>>> sense when speaking with Uwe at the committer workshop in January was that,
>>>> at least for Lucene, this fell in the latter bucket (e.g. not enough).
>>>>
>>>> Maurizio
>>>>
>>>>
>>>> Kind regards
>>>> Johanmes
>>>>
>>>> Maurizio Cimadamore <maurizio.cimadamore at oracle.com> schrieb am Mi.,
>>>> 22. Juli 2020, 11:51:
>>>>
>>>>>
>>>>> On 22/07/2020 00:44, Johannes Lichtenberger wrote:
>>>>>
>>>>> So, it likely will not be part of Java 15, right?
>>>>>
>>>>> Right.
>>>>>
>>>>>
>>>>> I'm running into the same problem, when I'm sharing a transaction
>>>>> (only one thread ever accesses it) with another thread and the other thread
>>>>> closes the transaction. Same problem with the segment close()-method.
>>>>>
>>>>> Are you too also using mapped segments? Or just plain native segments
>>>>> allocated with MemorySegment::allocateNative?
>>>>>
>>>>> Thanks
>>>>> Maurizio
>>>>>
>>>>>
>>>>> kind regards
>>>>> Johannes
>>>>>
>>>>> Am Mi., 22. Juli 2020 um 01:24 Uhr schrieb Maurizio Cimadamore <
>>>>> maurizio.cimadamore at oracle.com>:
>>>>>
>>>>>>
>>>>>> On 21/07/2020 22:51, NekoCaffeine wrote:
>>>>>> >>
>>>>>> https://gist.github.com/mcimadamore/128ee904157bb6c729a10596e69edffd
>>>>>> > This is a great example.
>>>>>> >
>>>>>> >> The *right* solution from an API perspective is to find a way to
>>>>>> disable confinement in a way that works
>>>>>> > I very much agree with this point, which is exactly what I want to
>>>>>> express.
>>>>>> >
>>>>>> > So from a design rather than implementation perspective, should I
>>>>>> expect a new way to disable thread checking or should I use
>>>>>> CustomMappedSegment?
>>>>>>
>>>>>> So the crux of the issue here is that there's no Runnable that will
>>>>>> unmap a given mapped address. But such a Runnable can be implemented
>>>>>> in
>>>>>> several ways:
>>>>>>
>>>>>> * with plain JNI
>>>>>> * using the ForeignLinker API (although that is not part of JDK 15 -
>>>>>> but
>>>>>> is available in the Panama repo)
>>>>>> * going all in and re-implement mapped segment (similar to what I've
>>>>>> done with CustomMappedSegment)
>>>>>>
>>>>>> I think from an API perspective, if we manage to solve the
>>>>>> confinement
>>>>>> problem, then I expect the memory segment API will have an extra
>>>>>> method
>>>>>> to either turn a confined segment into an unconfined one, or
>>>>>> vice-versa.
>>>>>> So you will be able to just create your mapped segment using
>>>>>> mapFromPath
>>>>>> and then just 'share' it (and make it unconfined).
>>>>>>
>>>>>> Hope this helps.
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>> >
>>>>>> > Thanks,
>>>>>> >
>>>>>> > NekoCaffeine
>>>>>>
>>>>>