BSTRByReference constructor loses reference to allocated memory

[dbwiddis]

1. Version of JNA and related jars: 5.6.0
2. Version and vendor of the java virtual machine: Reproduced on Oracle JDK 1.8.0_221, OpenJDK 11 and OpenJDK 15.0.1
3. Operating system: Reproduced on Windows 7, macOS 11 and Linux (Ubuntu 18)
4. System architecture: 32 bit Windows (Intel64), 64 bit macOS (Intel64), 64 bit Linux (ARM)
5. Complete description of the problem:

The process of instantiating a BSTRByReference from an existing BSTR stores the peer value of the BSTR's pointer, but does not keep a reference to the actual BSTR used to create it:

public void setValue(BSTR value) {
    this.getPointer().setPointer(0, value.getPointer());
}

The pointed-to value is tied to the Memory backing the BSTR, and that native memory allocation can be lost when the BSTR is garbage collected.

6. Steps to reproduce:

• This test case fails randomly: testPlatformToStrings(com.sun.jna.platform.ByReferencePlatformToStringTest, line 88)
• To reproduce the failure more reliably, add in some forced GC and time delay.

public static void main(String[] args) throws InterruptedException {
    BSTRByReference foo = new BSTRByReference(new BSTR("foo"));
    System.gc();
    System.gc();
    Thread.sleep(1000);
    System.out.println(foo.getValue().toString());
}

The toString can access unallocated memory, causing a crash, or memory allocated (and written to) by another JVM thread, producing undefined behavior. The above has resulted in each of the following:

• OutOfMemoryError if the memory read for BSTR size prompts creating too large an array
• NegativeArraySizeException if the memory read for BSTR size is negative
• Invalid Memory Access Error if JNA catches the problem
• Corruption of the text string, e.g., fo成
• SIGSEGV (# C [libsystem_platform.dylib+0x10a9] _platform_memmove$VARIANT$Haswell+0x29)
• JVM Crash

In addition, the getValue() method itself causes Invalid memory access if the pointer value is not populated, so should have some null checking involved.

BSTRByReference b = new BSTRByReference();
System.out.println(b.getValue());

Thoughts:

• At a minimum, we should clearly document the expectation the user keeps track of the BSTR allocation in the BSTRByReference javadocs (and that it's normally done using the Windows API).
• I'm not sure it's appropriate for users to be instantiating their own Memory-backed BSTR's in the first place. A BSTR should be created/allocated with SysAllocString().
• If we do keep user-instantiatable BSTRs we should probably alter BSTRByReference to maintain a reference to the BSTR object used to create it.