[foreign-memaccess/abi] where do unsafe operations live

[Maurizio Cimadamore]

On 22/10/2019 12:49, Jorn Vernee wrote:
> This sounds like a decent idea, but I'm wondering if having the class 
> be named ForeignUnsafe isn't already enough of a deterrent? The 
> barrier of having a single `requires` in a module-info file doesn't 
> seem much higher? I think if we just take away the need for 
> --add-exports, then the barrier is already pretty low, and we need to 
> make sure users understand if they are interacting with a safe or 
> unsafe API. Maybe we could do something with an @UnsafeAPI annotation 
> here (which IDEs/javac could warn about)? Or some javadoc note like 
> "This api is _unsafe_. It's use could result in putting the VM in an 
> invalid state, leading possibly to VM crashes".

I agree, but I think that moving the unsafe stuff in its own module is a 
precondition to what you describe. Note that a module will have its own 
javadoc, where we can document that users can depend on this at their 
own risk (and if the module is marked with some annotation, we could 
issue a mandatory warning when importing it).

While ForeignUnsafe might be a scary name as it is, I believe that users 
should have an option to decide on which module they want to depend on. 
There has to be a way to say "give me only the safe stuff, please" and 
if I accidentally refer to the unsafe stuff, a compiler error should be 
issued. I think that's valuable to have.

>
> Also, it seems a given that calling native code is unsafe. After all, 
> anything can happen in native code. So, would all of SystemABI be in 
> the unsafe module as well? 
True - but calling JNI code can be considered equally unsafe.
> I think it would be more descriptive to have a separate 
> jdk.incubator.foreign.abi module in that case, which is documented to 
> contain unsafe APIs. Basically, I'm wondering if there exists a more 
> useful/descriptive classification than just "safe" <-> "unsafe" for 
> that particular case.

I think there are two things happening here, for which the term unsafe 
might be used:

- calling native code can result in pretty much anything. That is 
already the case with JNI and I don't think SystemABI is any worse than 
that. Since calling JNI methods doesn't require any particular dance, 
I'd be concerned if, to create a native method handle, we forced users 
through some unsafe conundrum (also note, if a client  really want to 
protect against this, a security manager could guard library lookups and 
SystemABI calls)

- creating a pointer out of thin air _without_ even invoking a native 
method is something fundamentally new. And it is also unsafe, since you 
can just use the memory access API to dereference it and crash bang the VM.

So I don't think the two cases are created equal - at least not in the 
measure of how Java has always dealt with JNI.

>
> Jorn
>
> On 18/10/2019 19:44, Maurizio Cimadamore wrote:
>> Hi,
>> As I was using foreign-memaccess and foreign-abi in anger, it 
>> occurred to me that the current layout of the incubating module is a 
>> bit limiting in case the user wants to perform some unsafe operations.
>>
>> Right now, unsafe operations are defined in the class 'ForeignUnsafe' 
>> which lives in the (non-exported) jdk.incubating.foreign.unsafe package.
>>
>> Since the package is not exported, users that want to access unsafe 
>> stuff need to add a manual --add-exports option.
>>
>> This is fine, and, in fact, it was intended to have something that 
>> required an explicit opt-in, given that these are unsafe operations.
>>
>> That said, let's look at the situation from the perspective of a 
>> library writer: if I write a Java bindings, it is very likely that I 
>> might want to create MemoryAddress from longs, and the likes. But if 
>> I do that, I need to break into the jdk.incubating.foreign module, 
>> and _require that all my clients do the same_!
>>
>> I think this is excessively punitive. It's a good idea to have a 
>> sharp separation between "safe" and "unsafe" parts of the API - and 
>> it is also good if the "unsafe" parts of the API live in modules that 
>> are non-resolvable by default (e.g. you need to "require" them 
>> explicitly, or they won't be available).
>>
>> But if somebody is packaging a library in a module, it is kind of sad 
>> that there's no way for the library writer to depend on the 'unsafe' 
>> module, if he so wishes (and free the library clients from having to 
>> make this explicitly).
>>
>> All things considered, I think that a better structure would be to 
>> have _two_ modules:
>>
>> * jdk.incubating.foreign - usual, safe portion of the API
>> * jdk.incubating.foreign.unsafe - unsafe portion of the foreign API 
>> (requires jdk.incubating.foreign)
>>
>> This way, a library developer can just say "requires 
>> jdk.incubating.foreign.unsafe" and be done. The library clients will 
>> have to do nothing, assuming their module path is set up correctly.
>>
>> Thoughts?
>>
>> Maurizio
>>