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

Type safety error in fiber serialisation/TLS swapping code can cause weird crashes #147

Closed
mikehearn opened this issue Jan 6, 2016 · 1 comment

Comments

@mikehearn
Copy link
Contributor

Fiber defines the following fields:

private Object fiberLocals; private Object inheritableFiberLocals;

It is unclear to me why type safety is disabled for these fields, but it appears that normally they are meant to contain a ThreadLocalMap object, judging from Fiber.switchFiberAndThreadLocals()

However FiberSerializer changes the type of these fields with no explanation:

f.fiberLocals = f.fiberLocals != null ? filterThreadLocalMap(ThreadAccess.toMap(f.fiberLocals)).keySet().toArray() : null;
f.inheritableFiberLocals = f.inheritableFiberLocals != null ? filterThreadLocalMap(ThreadAccess.toMap(f.inheritableFiberLocals)).keySet().toArray() : null;

After this code runs f.fiberLocals is now an array, not a ThreadLocalMap. No explanation is provided in the code for the type switch. This causes a crash when debugging is enabled on the next run through switchFiberAndThreadLocals when it tries to print out the contents to the debug log.

@pron
Copy link
Contributor

pron commented Jan 6, 2016

It is unclear to me why type safety is disabled for these fields

Because ThreadLocalMap is package-private.

However FiberSerializer changes the type of these fields with no explanation

That's because ThreadLocalMap is not serializable.

on the next run through switchFiberAndThreadLocals

Ah, but in the original use-case for serialization, there would be no next run. Once a fiber is serialized, the original fiber was presumed to be discarded until a new one is recreated from the serialized representation.

It seems like fiber serialization needs to be retrofitted to allow your use-case. I don't think it's a big change by any means, but the current assumptions are wrong.

@pron pron closed this as completed in 6d01bae Jan 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants