关于 SessionInMemory 的一个 bug

zhaojun1998 commented 5 years ago

复现场景

开启 redisSessionDAO.setSessionInMemoryEnabled(true)
访问登录页，登录页有一个验证码，验证码通过工具生成，并调用 SecurityUtils.getSubject().getSession().setAttribute("captcha", "生成的验证码")。
登录时，调用 SecurityUtils.getSubject().getSession().getAttribute("captcha") 取出验证码进行校验。

在 SessionInMemoryEnabled 为 false 时，一切工作正常。当 SessionInMemoryEnabled 为 true 时，第三步获取到的验证码，并不一定是第二步设置的验证码。可能会出现错乱。

复现 GIF：

复现 GIF 对应的 demo 工程： https://cdn.jun6.net/shiro-redis-bug.zip

sika-code-cloud commented 5 years ago

这是因为使用了ThreadLocal后线程执行文笔没有执行remove的一个bug，不知道作者是否已经修复。

fattan9577 commented 5 years ago

首先，得明白SessionInMemoryEnabled参数时做什么的，由于shiro框架的关系，每次到来一个请求时，doReadSession就会执行多遍，由于在doReadSession中会读取redis。所以会造成性能的影响，作者引入了ThreadLocal，来将值存入ThreadLocal中，以减少redis请求次数。而这个参数就是是否选择使用TreadLocal
第二，变量DEFAULT_SESSION_IN_MEMORY_TIMEOUT,代表ThreadLocal中Session的存活时长，上述问题，是因为该时长设置过大，验证码点击频率过快造成的。造成这个现象的主要因素，是因为Servle容器t是采用多线程来执行请求的，每次执行代码的线程未必是同一个线程。而ThreadLocal是和线程绑定的，所以当你点击多次验证码后，其实每个验证码的值都存入了不同的TreadLocal中。当请求到来时，走doReadSession方法时会执行getSessionFromThreadLocal，尝试从Treadlocal中获取session，此时会判断当前线程绑定的Treadlocal中的SessionInMemory是否存活超过DEFAULT_SESSION_IN_MEMORY_TIMEOUT，如果超过就从当前线程中的treadlocal中的map中移除，并返回null，返回null后，doReadSession方法会继续执行，尝试从redis中读取值。所以当超时设置过大时，代码不会从redis中读值，而是从当前线程绑定的ThreadLocal中读值，造成错误。当超时设置小时，每次都会删除当前TreadLocal中的值，再从redis中读值。所以应权衡利弊，当session中信息频繁改动时，应将超时时间设小

sika-code-cloud commented 5 years ago

关于你说设置得超时时间过大得问题，使用得默认设置就是1S，难道1S也过长吗？也许一个请求链路都不一定走完。对于同一个请求，是同一个线程得，不同得请求是不同得线程拿到的ThreadLocal自然也不一样，而每次都会首先从redis里面读出来后缓存到ThreadLocal，但是假如不同得用户在第二次请求得时候拿到上一个用户得线程，那这个时候是不是会有问题？

------------------ 原始邮件 ------------------ 发件人: "fattan9577"notifications@github.com; 发送时间: 2019年5月7日(星期二) 晚上8:38 收件人: "alexxiyang/shiro-redis"shiro-redis@noreply.github.com; 抄送: "戴生"466608943@qq.com; "Comment"comment@noreply.github.com; 主题: Re: [alexxiyang/shiro-redis] 关于 SessionInMemory 的一个 bug (#97)

首先，得明白SessionInMemoryEnabled参数时做什么的，由于shiro框架的关系，每次到来一个请求时，doReadSession就会执行多遍，由于在doReadSession中会读取redis。所以会造成性能的影响，作者引入了ThreadLocal，来将值存入ThreadLocal中，以减少redis请求次数。而这个参数就是是否选择使用TreadLocal

第二，变量DEFAULT_SESSION_IN_MEMORY_TIMEOUT,代表ThreadLocal中Session的存活时长，上述问题，是因为该时长设置过大，验证码点击频率过快造成的。造成这个现象的主要因素，是因为Servle容器t是采用多线程来执行请求的，每次执行代码的线程未必是同一个线程。而ThreadLocal是和线程绑定的，所以当你点击多次验证码后，其实每个验证码的值都存入了不同的TreadLocal中。当请求到来时，走doReadSession方法时会执行getSessionFromThreadLocal，尝试从Treadlocal中获取session，此时会判断当前线程绑定的Treadlocal中的SessionInMemory是否存活超过DEFAULT_SESSION_IN_MEMORY_TIMEOUT，如果超过就从当前线程中的treadlocal中的map中移除，并返回null，返回null后，doReadSession方法会继续执行，尝试从redis中读取值。所以当超时设置过大时，代码不会从redis中读值，而是从当前线程绑定的ThreadLocal中读值，造成错误。当超时设置小时，每次都会删除当前TreadLocal中的值，再从redis中读值。所以应权衡利弊，当session中信息频繁改动时，应将超时时间设小

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

fattan9577 commented 5 years ago

首先，作者这里设计肯定是有弊端的。其次上方出问题的项目中，点击一次验证码的时间远远小于1s，而且那位用户将SessionInmemoryTimeOut的超时时间设置为了10s，这也就能解释为什么取的值是错误的。第三点，每次取值是先去ThreadLocal中取，如果ThreadLocal中没有，或者有值但超过存活时长，再从redis中取。第四，至于你说的不同的用户拿到同一个线程的问题，线程是交由线程池管理的，这里不做讨论了吧。

sika-code-cloud commented 5 years ago

我这边碰到得情况应该就是线程池得原因，导致若请求频繁得时候会出现下一个请求拿到上一个请求得ThreadLocal中得值，若在请求结束之后remove调ThroadLocal中应该就可以把这个问题规避掉

------------------ 原始邮件 ------------------ 发件人: "fattan9577"notifications@github.com; 发送时间: 2019年5月7日(星期二) 晚上9:02 收件人: "alexxiyang/shiro-redis"shiro-redis@noreply.github.com; 抄送: "戴生"466608943@qq.com; "Comment"comment@noreply.github.com; 主题: Re: [alexxiyang/shiro-redis] 关于 SessionInMemory 的一个 bug (#97)

首先，作者这里设计肯定是有弊端的。其次上方出问题的项目中，点击一次验证码的时间远远小于1s，而且那位用户将SessionInmemoryTimeOut的超时时间设置为了10s，这也就能解释为什么取的值是错误的。第三点，每次取值是先去ThreadLocal中取，如果ThreadLocal中没有，或者有值但超过存活时长，再从redis中取。第四，至于你说的不同的用户拿到同一个线程的问题，线程是交由线程池管理的，这里不做讨论了吧。

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

fattan9577 commented 5 years ago

作者用ThreadLocal的目的就是为了复用，减少redis请求次数。如果请求很频繁，频繁改动session中的值，那就将SessionInMemoryEnabled 设为false，

sika-code-cloud commented 5 years ago

老哥，请求频繁是很正常得呀，但是请求并不是为了改动session中得值，而是为了读取session中的值

------------------ 原始邮件 ------------------ 发件人: "fattan9577"notifications@github.com; 发送时间: 2019年5月7日(星期二) 晚上9:25 收件人: "alexxiyang/shiro-redis"shiro-redis@noreply.github.com; 抄送: "戴生"466608943@qq.com; "Comment"comment@noreply.github.com; 主题: Re: [alexxiyang/shiro-redis] 关于 SessionInMemory 的一个 bug (#97)

作者用ThreadLocal的目的就是为了复用，减少redis请求次数。如果请求很频繁，频繁改动session中的值，那就将SessionInMemoryEnabled 设为false，

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

fattan9577 commented 5 years ago

比方用户登录一次，将基本信息放在session不经常改的，SessionInMemory值可以设大一些，如果像上例，验证码1秒10次这种，每次都改session，放在ThreadLocal中没有意义

sika-code-cloud commented 5 years ago

老哥，加个微信细聊啊。dai5527153

------------------ 原始邮件 ------------------ 发件人: "fattan9577"notifications@github.com; 发送时间: 2019年5月7日(星期二) 晚上9:29 收件人: "alexxiyang/shiro-redis"shiro-redis@noreply.github.com; 抄送: "戴生"466608943@qq.com; "Comment"comment@noreply.github.com; 主题: Re: [alexxiyang/shiro-redis] 关于 SessionInMemory 的一个 bug (#97)

比方用户登录一次，将基本信息放在session不经常改的，SessionInMemory值可以设大一些，如果像上例，验证码1秒10次这种，每次都改session，放在ThreadLocal中没有意义

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

aroundsm commented 4 years ago

我遇到了相同问题，场景是用户扫码支付，session比较多，发现运行一段时间内存一直无法被gc。排查后看到大量http线程持有sessionMap，中有大量过期的SessionInMemory对象。分析应该是由于请求是线程池处理的，每次请求由不同线程处理，导致session被放入缓存后并不一定会被取出，也就无法判断是否过期。从而导致内存泄露。

alexxiyang commented 4 years ago

确实有存在内存泄漏问题。已经在3.3.0中改进了。虽然比较好的做法是启动一个Worker负责定时清理过期的SessionInThread，但是作为一个插件，最好不要单独启动线程，所以我在RedisSessionDAO的每一个接口方法处增加了检测过期SessionInThread的代码

ilaotan commented 4 years ago

期待3.3.0 最近在排查线上一个内存泄漏的bug，查到咱这个项目了，线上用的是3.1.0

alexxiyang / shiro-redis

关于 SessionInMemory 的一个 bug #97