Race-Bug the Session-Implementation

[knotenpunkt]

https://github.com/vert-x3/vertx-web/blob/bfe12934ec1251928c4744f41c898ee2d0dfc101/vertx-web/src/main/java/io/vertx/ext/web/sstore/impl/LocalSessionStoreImpl.java#L104

we discussed in the past already about this bug. Maybe you have a solution for this.

Its a race-bug, Sometimes there can be two or more unrelated Sessions be written, if different eventloops are on the same time on line 110 for example.

So the failedFuture is not always the case in cases where it should always be triggered and finish the execution.

The same problem we have in the clusteredSession; https://github.com/vert-x3/vertx-web/blob/bfe12934ec1251928c4744f41c898ee2d0dfc101/vertx-web/src/main/java/io/vertx/ext/web/sstore/impl/ClusteredSessionStoreImpl.java#L136

The version check reduces the race, but does not completely eliminate it. However, this version check suggests that it would be safe. I think we need some 100% thread-safe solution at least for the local session. At the cluster session, we should have a note in the documentation that this is not 100% secure, or optionally offer a 100% solution there as well, which then clearly works a little bit slower than the current one, and therefore does not replace it additionally

lg knotenpunkt

[pmlopes]

Session stores are not expected to be transactional. In 4 we have added a new api method: flush that can be called by the user and will await for the data to be persisted to the store.

If you have a transactional store then you can handle it at the store, rollback or recover on error.

Perhaps the improvement we can do, is to document this example.

As a good exercise or a pull request from the community a session store backed by sql client, could be a good addition. Just like the redis one.

Would you be interested in contributing such store, I could assist you with reviews.

[pmlopes]

Closing as the current implementation gives users a best effort in ensuring that state is correct. For better control use flush and a relational database as backend.