UI multi-session I18N resets after random time because of WeakHashMap

[printomi]

Issue Description

In multi-session mode, one could set I18N separately for each session. However, I18N setting resets to default, after some time (I observed 10-20 seconds).

This may be because I choose (in #7185 ) WeakHashMap to store I18N setting for sessions, here:
https://github.com/deeplearning4j/deeplearning4j/blob/master/deeplearning4j/deeplearning4j-ui-parent/deeplearning4j-play/src/main/java/org/deeplearning4j/ui/i18n/DefaultI18N.java#L53

JavaDoc for WeakHashMap at https://docs.oracle.com/javase/8/docs/api/index.html?java/util/WeakHashMap.html says this:

An entry in a WeakHashMap will automatically be removed when its key is no longer in ordinary use. More precisely, the presence of a mapping for a given key will not prevent the key from being discarded by the garbage collector, that is, made finalizable, finalized, and then reclaimed.

Since the keys of this map (String sessionId) is created in a Play Actor thread that calls MultiSessionI18NRoute, key Strings can be cleaned up by the garbage collector, after MultiSessionI18NRoute returns a Response.

This behavior is not intended. Changing DefaultI18N.sessionInstances from WeakHashMap to HashMap seems to solve the issue.

WeakHashMap in TrainModule

I used WeakHashMap in DefaultI18N.sessionInstances to follow the practice of TrainModule.knownSessionIDs. However, the later gets cleaned up in onDetach(...), so there is no need to use weak keys there. Moreover, the references to the mapping key (sessionId) in StatsStorage prevents the GC from removing any mapping, so WeakHashMap is useless in TrainModule. I would change this also to HashMap.

Version Information

• Deeplearning4j version: 1.0.0-SNAPSHOT
• platform information (OS, etc): Windows 10, Java 8

Contributing

I would create a PR with my solution.

I have manually tested the solution by running TestPlayUIMultiSession.testUIAutoAttachDetach(), opening a session in the browser, changing language, and refreshing the page for 1 minute.

Do you agree that using HashMap will be right?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.