[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
@michaelandrepearce I added a test to encode / decode in multi-thread. it 
should be enough.


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
@michaelandrepearce Someone has summoned me? If you repeat my name 3 times 
I'll disappear (cit: Beetlejuice)


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
re 2.0.0 and 2.6.x i fully blame @franz198 , little speed deamon! ;) :) 


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
re validBuffer im just wondering to simply avoid any possible future 
issue all it takes is a year or two's time and we all forget about this and 
someone implements something...


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
about previously from 2.0.0 I can only think that we have made improvements 
that probably allowed more load. I ran the same test against 2.0.0 and it fails 
as well.

Prior to 2.0.0 we would always copy the buffer during send and duplicate 
it. I tried to create a single buffer upon receiving and call saveToBuffer 
using the read-only-buffer.

During the process I missed this race. Even though I tested it quite a lot.

Regarding validBuffer, The application is single threaded upon receiving a 
message. We only need to validate it later during encode, which I added the 
synchronize. So i think it's ok?


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-29 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
@clebertsuconic did you see this?


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-26 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
Another thing ive noticed is currently where validBuffer is changed its not 
protected its only volatile, so you could enter syncronized method do the 
validBuffer check, but then subsequently because the validBuffer isn't 
protected it could change, before you exit the synced method.


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-25 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
@michaelandrepearce I will merge this now.. but if you could still review 
it after I merged it? we can send further commits. 


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-25 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
This is now ready to be merged.. (I had a word WIP on the name here before);


---


[GitHub] activemq-artemis issue #2396: ARTEMIS-2149 Protecting message.sendBuffer fro...

2018-10-25 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2396
  
@michaelandrepearce  can you review this one please.


---