The document you linked is not a specification itself
and as such is not binding. Do you have an actual specification bits that
prove this?

I went looking into servlet specification[1] and asked someone from WFLY
who has more expertise in this field to also take a look.

None of us found anything stating that you are required to call setAttribute()
every time you change mutable attribute.

In fact there is very little about attributes (or "replication triggers"
for attributes) and you could say this is grey area.

To me it still makes way more sense to distinguish between mutable/immutable
attributes and for mutable ones you should set "dirty" state
even on read, not just write.

Matej

_____________________________________________________________________________________

[1] 
https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf

----- Original Message -----

> From: "Benjamin Confino" <BENJAMIC@uk.ibm.com>

> To: "Allan Zhang" <zhang@ca.ibm.com>

> Cc: "Matej Novotny" <manovotn@redhat.com>, "Shinji
Ohtsuka" <EB92769@jp.ibm.com>, "Emily Jiang"

> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org

> Sent: Thursday, May 7, 2020 12:41:01 PM

> Subject: RE: [weld-dev] Propagation of org.jboss.weld.context.ConversationContext.conversations
through session

> failover

> 

> Hello

> 

> Based on Allan Zhang's comments I have closed my pull requests and
opened

> this one 
https://github.com/weld/core/pull/1990

 which is a better match

> for the recommendations in the sessionmanagement spec. I still have
not

> heard back from my customer so I will compile an updated test fix
and

> chase them up. If they approve, or if they do not reply soon I will
create

> pull requests for the other branches.

> 

> Regards

> Benjamin

> 

> 

> 

> From:   Allan Zhang/Toronto/IBM

> To:     Matej Novotny <manovotn@redhat.com>

> Cc:     Benjamin Confino <BENJAMIC@uk.ibm.com>, Shinji
Ohtsuka

> <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>,

> weld-dev@lists.jboss.org

> Date:   06/05/2020 17:42

> Subject:        Re: [EXTERNAL] Re: [weld-dev]
Propagation of

> org.jboss.weld.context.ConversationContext.conversations through session

> failover

> 

> 

> 

> >Shouldn't Liberty consider (at least after restart) any access
to a

> collection-based session property as an action marking that property

> "dirty"?

> 

> Liberty already has write option of ALL_SESSION_ATTRIBUTES to mark
the

> property "dirty" but customer wont use it for performance
reasons.

> 

> The PR fix is following the JEE spec where property "dirty"
require

> setAttribute() call.

> 
https://docs.oracle.com/cd/E13924_01/coh.340/e13819/sessionmanagement.htm

> 

> As a general rule, all session attributes should be treated as immutable

> objects if possible. This ensures that developers are consciously
aware

> when they change attributes. With mutable objects, modifying attributes

> often requires two steps: modifying the state of the attribute object,
and

> then manually updating the session with the modified attribute object
by

> calling javax.servlet.http.HttpSession.setAttribute(). This means
that

> your application should always call setAttribute() if the attribute
value

> has been changed, otherwise, the modified attribute value will not

> replicate to the backup server.

> 

> Thanks

> Allan Zhang

> 

> 

> 

> 

> From:   Matej Novotny <manovotn@redhat.com>

> To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> Cc:     Shinji Ohtsuka <EB92769@jp.ibm.com>, Emily
Jiang

> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, Allan Zhang

> <zhang@ca.ibm.com>

> Date:   2020-05-06 07:52 AM

> Subject:        [EXTERNAL] Re: [weld-dev] Propagation
of

> org.jboss.weld.context.ConversationContext.conversations through session

> failover

> 

> 

> 

> Hi,

> 

> thanks for explanation, I understand the flow now.

> 

> However, your suggested PR still looks like a workaround more than
a fix

> to me.

> 

> I doubt Weld is the only case where this will emerge. You can easily
store

> any collection in session and expect it to behave like we do.

> So in the very least, anything of type java.util.Collection that is
stored

> in session is susceptible to this behaviour.

> Shouldn't Liberty consider (at least after restart) any access to
a

> collection-based session property as an action marking that property

> "dirty"?

> 

> Regards

> Matej

> 

> ----- Original Message -----

> > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com>

> > To: "Matej Novotny" <manovotn@redhat.com>

> > Cc: "Shinji Ohtsuka" <EB92769@jp.ibm.com>, "Emily
Jiang"

> <EMIJIANG@uk.ibm.com>, weld-dev@lists.jboss.org, "Allan

> > Zhang" <zhang@ca.ibm.com>

> > Sent: Tuesday, May 5, 2020 3:59:30 PM

> > Subject: RE: [weld-dev] Propagation of

> org.jboss.weld.context.ConversationContext.conversations through session

> > failover

> > 

> > Hello Matej

> > 

> > No worries about the delay, my customer has not responded so
you are

> ahead

> > of schedule. You are correct, the purpose of this PR is to trigger
the

> > "dirty" state in liberty's session storage. Let me
try and describe the

> > flow again and hopefully it will be clearer:

> > 

> > 

> > A1) The application is started for the first time. A conversation
map is

> > created and stored into the session context. This marks it as
dirty.

> > A2) The application puts stuff into the map. Lets say this leaves
the

> map

> > in State A

> > A3) The server is told to shut down.

> > A4) Liberty sees the map is dirty and saves map State A to persistent

> > storage.

> > 

> > B1) The server starts again.

> > B2) Weld retrieves the conversation map (State A) from persistent

> storage.

> > Since it already has a map it does not put anything into session

> storage.

> > Thus the map is clean.

> > B3) The application does more stuff, moving the map to (State
B).

> > B3) The server is told to shut down.

> > B4) Liberty sees the map is clean, it doesn't transfer anything
to

> > persistent storage.

> > 

> > C1) The server starts again.

> > C2) Weld retrieves the conversation map (State A). This is the
wrong

> > state.

> > 

> > 

> > My PR changes B2 so that Weld will set the same map back into
session

> > storage. Then in B4 liberty will see the map is dirty and save
State B

> to

> > storage, ensuring the correct state is retrieved at C2.

> > 

> > We do have alternative solutions, for example you can configure
Liberty

> to

> > store everything regardless of whether it is dirty or not. However
this

> > has performance implications and so my customer does not want
to use

> that

> > option.

> > 

> > Regards

> > Benjamin

> > 

> > 

> > 

> > 

> > From:   Matej Novotny <manovotn@redhat.com>

> > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > Cc:     weld-dev@lists.jboss.org, Allan Zhang <zhang@ca.ibm.com>,
Shinji

> > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>

> > Date:   05/05/2020 12:17

> > Subject:        [EXTERNAL] Re: [weld-dev]
Propagation of

> > org.jboss.weld.context.ConversationContext.conversations through
session

> > failover

> > 

> > 

> > 

> > I finally found some time to look into this, it took longer than
I

> > anticipated, sorry for that.

> > 

> > Firstly, I am not quite following the flow of the reproducer
you

> > described.

> > Weld creates the conversation map and stores in into the session
on

> > creation. We then retrieve this map from session whenever needed
and

> store

> > additional things into it.

> > All the while we use the same map (the same reference, or object
if you

> > will)....so Liberty should see the same state in it, right? Or
am I

> > missing something obvious?

> > 

> > I am not familiar with how Liberty deals with server shutdown
and how it

> > stores session attributes, but i would expect that if you just
grab the

> > existing map from there, it will be up to date.

> > The PRs you sent seem superfluous to me - you are overriding
the

> attribute

> > with exactly the same thing. probably just to trigger "dirty"
state in

> > your session storage?

> > 

> > Regards

> > Matej

> > 

> > ----- Original Message -----

> > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com>

> > > To: "Matej Novotny" <manovotn@redhat.com>

> > > Cc: weld-dev@lists.jboss.org, "Allan Zhang" <zhang@ca.ibm.com>,

> "Shinji

> > Ohtsuka" <EB92769@jp.ibm.com>, "Emily Jiang"

> > > <EMIJIANG@uk.ibm.com>

> > > Sent: Wednesday, April 29, 2020 11:56:43 AM

> > > Subject: RE: [weld-dev] Propagation of

> > org.jboss.weld.context.ConversationContext.conversations through
session

> > > failover

> > > 

> > > Thank you for the heads up. When it's time to think about
delivery

> I'll

> > be

> > > sure to create a PR against 3.1 and I presume you'd like
me to leave

> > > master alone for now?

> > > 

> > > Regards

> > > Benjamin

> > > 

> > > 

> > > 

> > > From:   Matej Novotny <manovotn@redhat.com>

> > > To:     Benjamin Confino <BENJAMIC@uk.ibm.com>

> > > Cc:     weld-dev@lists.jboss.org, Allan Zhang
<zhang@ca.ibm.com>,

> Shinji

> > > Ohtsuka <EB92769@jp.ibm.com>, Emily Jiang <EMIJIANG@uk.ibm.com>

> > > Date:   29/04/2020 09:51

> > > Subject:        [EXTERNAL] Re: [weld-dev]
Propagation of

> > > org.jboss.weld.context.ConversationContext.conversations
through

> session

> > > failover

> > > 

> > > 

> > > 

> > > Hi,

> > > 

> > > I'll take a look later today.

> > > Note that master branch is no longer Weld 3.x, it is 4.x
(Jakarta EE

> 9)

> > > and the CI there is going bonkers yet as I am in the middle
of

> changing

> > > it.

> > > If you want to file a PR against Weld 3, you can use 3.1
branch for

> > that.

> > > 

> > > Regards

> > > Matej

> > > 

> > > ----- Original Message -----

> > > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com>

> > > > To: weld-dev@lists.jboss.org

> > > > Cc: "Allan Zhang" <zhang@ca.ibm.com>,
"Shinji Ohtsuka"

> > > <EB92769@jp.ibm.com>, "Emily Jiang" <EMIJIANG@uk.ibm.com>

> > > > Sent: Tuesday, April 28, 2020 2:31:44 PM

> > > > Subject: [weld-dev] Propagation of

> > > org.jboss.weld.context.ConversationContext.conversations
through

> session

> > > failover

> > > > 

> > > > Hello weld

> > > > 

> > > > I had a customer report that they were getting conversation
not

> found

> > > > exceptions when restarting their server and visiting
a url with a

> > ?cid=1

> > > > suffix.

> > > > 

> > > > After investigation I believe the issue is that weld
was acquiring

> > it's

> > > > ConversationContext.conversations from the session
database via

> > > > com.ibm.ws.session.store.db.DatabaseSession.getMultiRowAppData().

> Once

> > > weld

> > > > had retrieved the conversations map it would then decide
that since

> > the

> > > map

> > > > was already in the session attributes there was no
need to put it

> back

> > > into

> > > > the attributes.

> > > > 

> > > > This means that Liberty did not realise the conversations
map had

> been

> > > > updated, and did not store it's updated state into
the database when

> > the

> > > > server shut down again.

> > > > 

> > > > I have submitted a pair of pull requests that asks
weld to mark the

> > > > conversation map as dirty upon access - this behaviour
is gated

> behind

> > > > ConfigurationKey.RESET_HTTP_SESSION_ATTR_ON_BEAN_ACCESS
- I have

> > tested

> > > it

> > > > locally and it works. The next step is to prepare a
test fix for the

> > > > customer to verify. However I wanted to send you this
quick note to

> > keep

> > > you

> > > > in the loop.

> > > > 

> > > > Regards

> > > > Benjamin

> > > > Unless stated otherwise above:

> > > > IBM United Kingdom Limited - Registered in England
and Wales with

> > number

> > > > 741598.

> > > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire

> PO6

> > > 3AU

> > > > 

> > > > _______________________________________________

> > > > weld-dev mailing list

> > > > weld-dev@lists.jboss.org

> > > > 

> > > 

> > 

> 
https://lists.jboss.org/mailman/listinfo/weld-dev

> 

> > 

> > > 

> > > 

> > > 

> > > 

> > > 

> > > Unless stated otherwise above:

> > > IBM United Kingdom Limited - Registered in England and Wales
with

> number

> > > 741598.

> > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6

> > 3AU

> > > 

> > 

> > 

> > 

> > 

> > Unless stated otherwise above:

> > IBM United Kingdom Limited - Registered in England and Wales
with number

> > 741598.

> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6

> 3AU

> > 

> 

> 

> 

> 

> 

> Unless stated otherwise above:

> IBM United Kingdom Limited - Registered in England and Wales with
number

> 741598.

> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU

> 