On 7/12/2020 5:44 pm, Chris Plummer wrote:
On Mon, 7 Dec 2020 06:27:20 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 1560:

1558:              * garbage collected while the VM is suspended.
1559:              */
1560:             commonRef_pinAll();

Can we have multiple VM.suspend calls? The  suspendAllCount seems to suggest that. In 
which case shouldn't we only pin on the 0->1 transition, and only unpin on the 
1->0 transition?

That was something I pointed out in the pre-review, and it has been addressed 
in `commonRef_pinAll/unpinAll`:

`568         if (gdata->pinAllCount == 1) {`
`618         if (gdata->pinAllCount == 0) {`

Okay. I would not have handled it at that level, but would have had
pinAll/unpinAll operate unconditionally, but the calls to those methods
being conditional based on the suspendAllCount.

David

Well, that's assuming `pinAll()` will only ever be used by by `suspendAll()`. 
One could imaging a future use, such as if `VirtualMachine.disableCollection()` 
were ever to be added.

Not really. I consider pinAll should pin-all as the name implies. The question of when to pin should be handled by the caller of pinAll. If there were ever to be a second reason to pinAll then you would have to decide what semantics that has: does it maintain a count, or is it like thread suspension.

David

-------------

PR: https://git.openjdk.java.net/jdk/pull/1595

Reply via email to