RFR: 8373411: Crash when PrintSharedArchiveAndExit is enabled but shared heap is disabled

[Stefan Karlsson]

On Thu, 11 Dec 2025 09:38:56 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> > Personally, I think it is redundant, because the second assert would catch a failure of the first. But if you all think this is better, then I'll add it.
> 
> Meh, can go either way. I won't quibble for this patch.
> 
> Honestly, my expectation is that when `HeapShared::is_loading_mapping_mode() == true`, then `HeapShared::is_loading() == true`, and when `is_loading_mapping_mode() == false`, then `is_loading()` is undefined/dont-care value. In that sense, current code that checks only `HeapShared::is_loading_mapping_mode()` would have been correct already. But I am guessing you don't want to miss a bug when `is_loading_mapping_mode()` returns `false`, just because we have a lifecycle bug somewhere and loading mode is uninitialized. 

Yes. We have had a few of those bugs at the end of the development cycle for the object streaming JEP.

> OTOH, it is literally the same as checking `is_loading()` explicitly like your PR does, so it gains us nothing safety-wise, and only confuses ourselves. Maybe rethinking the state machinery a bit here would be a good cleanup.

I think Erik had it more like you suggest, but if I understood Ioi correctly he wanted things to be more explicit, and this is what we ended up with. I don't mind if someone wants to take a stab at restructuring this, but I'm not personally volunteering to do this. I would rather take the time to make other refactoring and renaming that we felt would benefit the heap sharing code, and we had hope to get to it after the JEP was delivered. However, the reality is that we're all prioritizing other work at the moment so I don't know if / when we'll get to that.

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

PR Comment: https://git.openjdk.org/jdk/pull/28741#issuecomment-3641121965