camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Onder SEZGIN <ondersez...@gmail.com>
Subject Re: camel git commit: CAMEL-11609:thread-safety if headers get modified on the fly
Date Fri, 28 Jul 2017 11:50:13 GMT
Hi,

I say 0 for my commit :)
Because it is currently not thread safe and sync will help to ensure it
thread-safe modification of the list..

As discussed in the issue and PR as well, with this commit we can ensure
thread-safety for the modification of the list

We may discuss if this is a right approach to evaluate the data format
design, but this is not the point i suppose.

Please see my comment. (link
<https://issues.apache.org/jira/browse/CAMEL-11609?focusedCommentId=16103900&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16103900>
)
and my comment in the PR as well. (link
<https://github.com/apache/camel/pull/1855#discussion_r130071814>)

On Fri, Jul 28, 2017 at 1:56 PM, Claus Ibsen <claus.ibsen@gmail.com> wrote:

> Hi
>
> Yeah the data format implementation of marshal / unmarshal should be
> written in a thread-safe manner. So I think the current code in
> univocity may be wrong, if it has some kind of shared state for some
> headers.
>
>
>
> On Fri, Jul 28, 2017 at 10:51 AM, Zoran Regvart <zoran@regvart.com> wrote:
> > Hi Cameleers,
> >
> > -1
> >
> > not sure that was the correct solution, perhaps `headers` field of
> > Marshaller needs to be treated as immutable. Hard to tell on such a
> > rarely used component what was the initial intent of `headers` field,
> > but I guess it's what all marshallers need to add, so mutating it on a
> > per write seems wrong.
> >
> > Any thoughts?
> >
> > zoran
> >
> > On Fri, Jul 28, 2017 at 10:27 AM,  <onders@apache.org> wrote:
> >> Repository: camel
> >> Updated Branches:
> >>   refs/heads/master 6d0fdee12 -> 27d6c83ec
> >>
> >>
> >> CAMEL-11609:thread-safety if headers get modified on the fly
> >>
> >>
> >> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
> >> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/27d6c83e
> >> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/27d6c83e
> >> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/27d6c83e
> >>
> >> Branch: refs/heads/master
> >> Commit: 27d6c83ecd392168e59971f5de8e4ad258a1f461
> >> Parents: 6d0fdee
> >> Author: onders86 <ondersezgin@gmail.com>
> >> Authored: Thu Jul 27 23:54:18 2017 +0300
> >> Committer: onders86 <ondersezgin@gmail.com>
> >> Committed: Fri Jul 28 11:27:06 2017 +0300
> >>
> >> ----------------------------------------------------------------------
> >>  .../org/apache/camel/dataformat/univocity/Marshaller.java    | 8
> ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >> ----------------------------------------------------------------------
> >>
> >>
> >> http://git-wip-us.apache.org/repos/asf/camel/blob/27d6c83e/
> components/camel-univocity-parsers/src/main/java/org/
> apache/camel/dataformat/univocity/Marshaller.java
> >> ----------------------------------------------------------------------
> >> diff --git a/components/camel-univocity-parsers/src/main/java/org/
> apache/camel/dataformat/univocity/Marshaller.java
> b/components/camel-univocity-parsers/src/main/java/org/
> apache/camel/dataformat/univocity/Marshaller.java
> >> index 5c113cb..e170c95 100644
> >> --- a/components/camel-univocity-parsers/src/main/java/org/
> apache/camel/dataformat/univocity/Marshaller.java
> >> +++ b/components/camel-univocity-parsers/src/main/java/org/
> apache/camel/dataformat/univocity/Marshaller.java
> >> @@ -46,7 +46,9 @@ final class Marshaller<W extends AbstractWriter<?>>
{
> >>       */
> >>      Marshaller(String[] headers, boolean adaptHeaders) {
> >>          if (headers != null) {
> >> -            this.headers.addAll(Arrays.asList(headers));
> >> +            synchronized (this.headers) {
> >> +                this.headers.addAll(Arrays.asList(headers));
> >> +            }
> >>          }
> >>          this.adaptHeaders = adaptHeaders;
> >>      }
> >> @@ -86,7 +88,9 @@ final class Marshaller<W extends AbstractWriter<?>>
{
> >>          Map<?, ?> map = convertToMandatoryType(exchange, Map.class,
> row);
> >>          if (adaptHeaders) {
> >>              for (Object key : map.keySet()) {
> >> -                headers.add(convertToMandatoryType(exchange,
> String.class, key));
> >> +                synchronized (headers) {
> >> +                    headers.add(convertToMandatoryType(exchange,
> String.class, key));
> >> +                }
> >>              }
> >>          }
> >>
> >>
> >
> >
> >
> > --
> > Zoran Regvart
>
>
>
> --
> Claus Ibsen
> -----------------
> http://davsclaus.com @davsclaus
> Camel in Action 2: https://www.manning.com/ibsen2
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message