Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insulate BeanInfo from segmentation errors at runtime with GraalVM [SPR-17138] #21675

Closed
spring-projects-issues opened this issue Aug 7, 2018 · 3 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression)

Comments

@spring-projects-issues
Copy link
Collaborator

Dave Syer opened SPR-17138 and commented

GraalVM (and possibly other reflection sensitive tools) barfs really badly on the standard BeanInfo:

 ./target/bunc 

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::                        

[ [ SubstrateSegfaultHandler caught signal 11 ] ]

General Purpose Register Set Values: 

  RAX 000055f1af7d6478
  RBX 00007f00afa6e1e8
  RCX 00007f00afa6e1e8
  RDX 000055f1af51d7a8
  RBP 000055f1aeb6ba80
  RSI 0000000000000010
  RDI 000055f1af7d5d08
  RSP 00007fff306e0860
  R8  0000000000000010
  R9  00000000c0100701
  R10 00007f00afa6e158
  R11 000000000000000e
  R12 0000000000000010
  R13 0000000000000026
  R14 000000000000002e
  R15 000055f1b1409260
  EFL 0000000000010246
  RIP 000055f1aead90fc
  ...

etc.

I guess it might just be a bug in GraalVM (oracle/graal#522), but it is also possible to "fix" it by using a custom BeanInfo:

class SpringBootBeanInfo implements BeanInfo {

	private final PropertyDescriptor[] propertyDescriptors;

	public SpringBootBeanInfo(Class<?> beanClass) throws IntrospectionException {
		this.propertyDescriptors = extractPropertyDescriptors(beanClass);
	}

	private PropertyDescriptor[] extractPropertyDescriptors(Class<?> beanClass)
			throws IntrospectionException {
		Map<String, Method> getters = new LinkedHashMap<>();
		Map<String, Method> setters = new LinkedHashMap<>();
		Method[] methods = ReflectionUtils.getAllDeclaredMethods(beanClass);
		for (Method method : methods) {
			collectGetterSetterMethod(method, getters, setters);
		}
		List<PropertyDescriptor> descriptors = new ArrayList<>(methods.length);
		for (Map.Entry<String, Method> entry : getters.entrySet()) {
			String name = entry.getKey();
			Method getter = entry.getValue();
			Method setter = setters.remove(name);
			if (setter != null && !getter.getReturnType()
					.isAssignableFrom(setter.getParameterTypes()[0])) {
				setter = null;
			}
			descriptors.add(new SlimPropertyDescriptor(name, getter, setter));
		}
		for (Map.Entry<String, Method> entry : setters.entrySet()) {
			Method setter = entry.getValue();
			String name = entry.getKey();
			// System.err.println("**************** " + setter);
			descriptors.add(new SlimPropertyDescriptor(name, null, setter));
		}
		return descriptors.toArray(new SlimPropertyDescriptor[descriptors.size()]);
	}

	private void collectGetterSetterMethod(Method method, Map<String, Method> getters,
			Map<String, Method> setters) {
		int argSize = method.getParameterTypes().length;
		if (!Modifier.isStatic(method.getModifiers())
				&& Modifier.isPublic(method.getModifiers()) && argSize <= 1) {
			String name = method.getName();
			if (argSize == 0 && name.length() > 3 && name.startsWith("get")
					&& !name.equals("getClass")) {
				getters.putIfAbsent(name.substring(3), method);
			}
			else if (argSize == 0 && name.length() > 2 && name.startsWith("is")
					&& method.getReturnType() == boolean.class) {
				getters.putIfAbsent(name.substring(2), method);
			}
			else if (argSize == 1 && name.length() > 3 && name.startsWith("set")) {
				setters.putIfAbsent(name.substring(3), method);
			}
		}
	}

	@Override
	public BeanDescriptor getBeanDescriptor() {
		throw new UnsupportedOperationException();
	}

	@Override
	public EventSetDescriptor[] getEventSetDescriptors() {
		throw new UnsupportedOperationException();
	}

	@Override
	public int getDefaultEventIndex() {
		throw new UnsupportedOperationException();
	}

	@Override
	public int getDefaultPropertyIndex() {
		throw new UnsupportedOperationException();
	}

	@Override
	public MethodDescriptor[] getMethodDescriptors() {
		throw new UnsupportedOperationException();
	}

	@Override
	public BeanInfo[] getAdditionalBeanInfo() {
		throw new UnsupportedOperationException();
	}

	@Override
	public Image getIcon(int iconKind) {
		throw new UnsupportedOperationException();
	}

	@Override
	public PropertyDescriptor[] getPropertyDescriptors() {
		return this.propertyDescriptors;
	}

}

class SlimPropertyDescriptor extends PropertyDescriptor {

	private Method readMethod;
	private Method writeMethod;

	public SlimPropertyDescriptor(String name, Method getter, Method setter)
			throws IntrospectionException {
		super(name, getter, setter);
	}

	@Override
	public synchronized void setReadMethod(Method readMethod)
			throws IntrospectionException {
		this.readMethod = readMethod;
	}

	@Override
	public synchronized void setWriteMethod(Method writeMethod)
			throws IntrospectionException {
		this.writeMethod = writeMethod;
	}

	@Override
	public Method getReadMethod() {
		return readMethod;
	}

	@Override
	public Method getWriteMethod() {
		return writeMethod;
	}
}

The key is to override the getters and setters there in PropertyDescriptor.


Affects: 5.0.8

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

I think I would wait Graal issue to be closed before moving on that one.

@spring-projects-issues spring-projects-issues added status: waiting-for-triage An issue we've not yet triaged or decided on type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) and removed type: enhancement A general enhancement labels Jan 11, 2019
@dsyer
Copy link
Member

dsyer commented Jul 8, 2019

The Graal issue is closed. Do we need to do something here or not (it might not hurt to be more defensive)?

@sdeleuze
Copy link
Contributor

I think we can close it, the fix on GraalVM side seems to be enough.

@sdeleuze sdeleuze removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression)
Projects
None yet
Development

No branches or pull requests

3 participants