You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicemix.apache.org by "Ivan Pryvalov (JIRA)" <ji...@apache.org> on 2008/09/10 12:10:52 UTC

[jira] Created: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

CleanUp tempFiles-attachments is not thread-safe
------------------------------------------------

                 Key: SM-1569
                 URL: https://issues.apache.org/activemq/browse/SM-1569
             Project: ServiceMix
          Issue Type: Bug
         Environment: SMX-3.3
            Reporter: Ivan Pryvalov
             Fix For: 3.3




-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Ivan Pryvalov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45616#action_45616 ] 

Ivan Pryvalov commented on SM-1569:
-----------------------------------

Hi Lars!

I'm talking not about the same exchange in the same endpoint at one time.... This looks also somehow impossible to me...

But I'm talking about miltithreading. When we have one instance of some util-class (List, Map) and few threads to modify it (add, put etc).

In case ArrayList I think everyone knows example with bad using of methods "add" "get" "remove" in a few threads... 

Simple scenario (1 - thread one, 2- thread 2):

1.  add (element) ---> [ now size is 1]
2. size = getSize() ---> [return 1]
1. remove (0) ---> [now size is 0]
2. get( size-1) ---> ArrayIndexBoundException.

This example we can easy to reproduce....

But now we have HashMap..... If we think HashMap is thread-safe - that is WRONG. 
We call methods get, put, remove which influence on internal state of instance of HashMap. And in this case we CAN NOT be sure if we have the same INTERNAL STATE during running one thread.

I specially prepared simple example which illustrates it:


////// code

package test.hashmap;

import java.util.HashMap;
import java.util.Map;

public class TestHashMap {
	private Map<String, String> map = null;

	public TestHashMap(Map<String, String> map) {
		super();
		this.map = map;
	}

	public class TestOne implements Runnable {
		private String id = null;
		private Map<String, String> map = null;

		public TestOne(String id, Map<String, String> map) {
			super();
			this.id = id;
			this.map = map;
		}

		public void run() {
			map.put(id, id);
			String value = map.get(id);
			System.out.println("get(" + id + ")=" + value);

		}
	}

	public void runTest(int count) {
		for (int i = 0; i < count; i++) {
			new Thread(new TestOne(Integer.toString(i), map)).start();
		}
	}

	public static void main(String[] args) {
		Map<String, String> map = new HashMap<String, String>();
		TestHashMap testHashMap = new TestHashMap(map);
		testHashMap.runTest(1000);

	}

}


////// end code



After test sometimes in conlose I get something like following:

get(1)=1
get(0)=0
get(2)=2
get(3)=3
get(5)=5
get(7)=7
get(4)=4
.....
get(380)=380
get(381)=381
get(379)=379
get(384)=384
get(383)=383
get(386)=386
get(382)=null
get(388)=388
get(387)=387
get(385)=null
get(390)=390
get(389)=389
get(391)=391
get(392)=392
get(393)=393
get(395)=395
.....



get(382)=null  ------> it's BAD situdation... And we can not prognose it.... Sometimes it does not apper, but sometimes it is apper...

Truly yours,
Ivan Pryvvaov.


> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Guillaume Nodet (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Guillaume Nodet updated SM-1569:
--------------------------------

    Fix Version/s: servicemix-mail-2008.01

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3, servicemix-mail-2008.01
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars Heinemann updated SM-1569:
-------------------------------

    Affects Version/s:     (was: 3.3)
                       servicemix-mail-2008.01
        Fix Version/s:     (was: 3.3)

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: servicemix-mail-2008.01
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: servicemix-mail-2008.01
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Ivan Pryvalov (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Pryvalov updated SM-1569:
------------------------------

        Fix Version/s:     (was: 3.3)
    Affects Version/s: 3.3
          Component/s: servicemix-mail

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Ivan Pryvalov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45584#action_45584 ] 

Ivan Pryvalov commented on SM-1569:
-----------------------------------

class AbstractMailMarchaller is not thread-safe:

It means if I will send 2 excganges to some SU based on servicemix-mail component, after first exchange is processed, it is executed cleanUp method of AbstractMarshaller, and while executing second exchange we get "FileNotFoundException".

Schema:


1 Exchange:  -> AR SA  S   C



2 Exchange:   ------->  AR   SA     S    C


AR - AcceptionRequest
SA - SavingAttachments
S - Sending e-mail
C - CleanUp.

Troubles is in point 2Exchange.S  (after 1.Exchange.C) 





I would to replace 

    private List<File> temporaryFiles = new ArrayList<File>();

 with:

   private Map< String, List<File> > temporaryFilesMap = Collections.synchronizedMap(new HashMap<String, List<File> >());


and:

protected final void addTemporaryResource(String id, File tmpFile) {
        if (!this.temporaryFilesMap.containsKey(id))
            this.temporaryFilesMap.put(id, new ArrayList<File>());
        this.temporaryFilesMap.get(id).add(tmpFile);
    }

and:

    public final void cleanUpResources(String id) {
        List<File> list = this.temporaryFilesMap.get(id);
        if (list!=null){           
            for (File f : list) {
                f.delete();
            }
            list.clear();
            this.temporaryFilesMap.remove(id);
        }
    }


And do necessary changes in code from other classes (also it is needed to update tests to avoid NullPointerException, I mean following:  replace InOnly exchange = new InOnlyImpl() with  InOnly exchange = new InOnlyImpl("id")) 

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45615#action_45615 ] 

Lars Heinemann commented on SM-1569:
------------------------------------

Ivan,

I can't see how multiple threads can process the same message exchange in the 
same endpoint at one time. This looks somehow impossible to me ;)
The only thing I could imagine is that the hashmap internal algorithm is somehow disturbed by 
accessing / filling etc. from different threads in a not synchronized way. 

Regards
Lars

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Work started: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on SM-1569 started by Lars Heinemann.

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars Heinemann resolved SM-1569.
--------------------------------

    Fix Version/s: 3.3
       Resolution: Fixed

Applied the patch.
Thanks for pointoing this out, Ivan.

One remark:
I am not sure if it is really needed to have the map synchronized as normally dividing by message exchange id should be enough.
Anyway, I put in the code as you postet here.

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars Heinemann reassigned SM-1569:
----------------------------------

    Assignee: Lars Heinemann

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by Lars Heinemann <la...@compart.net>.
Ivan,

I can't see how multiple threads can process the same message exchange in the 
same endpoint. This looks somehow impossible to me ;)

Regards
Lars


Am Mittwoch 10 September 2008 14:10:52 schrieb Ivan Pryvalov (JIRA):
>     [
> https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.p
>lugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45592#action_4
>5592 ]
>
> Ivan Pryvalov commented on SM-1569:
> -----------------------------------
>
> >One remark:
> >I am not sure if it is really needed to have the map synchronized as
> > normally dividing by message exchange id should be enough.
>
> If we have multi-threading env, the only syncronized collection can give us
> ensure in safety. In case simple HashMap, behaviour of component can be
> unpredictable.
>
> > CleanUp tempFiles-attachments is not thread-safe
> > ------------------------------------------------
> >
> >                 Key: SM-1569
> >                 URL: https://issues.apache.org/activemq/browse/SM-1569
> >             Project: ServiceMix
> >          Issue Type: Bug
> >          Components: servicemix-mail
> >    Affects Versions: 3.3
> >         Environment: SMX-3.3
> >            Reporter: Ivan Pryvalov
> >            Assignee: Lars Heinemann
> >             Fix For: 3.3


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Ivan Pryvalov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45592#action_45592 ] 

Ivan Pryvalov commented on SM-1569:
-----------------------------------

>One remark:
>I am not sure if it is really needed to have the map synchronized as normally dividing by message exchange id should be enough.

If we have multi-threading env, the only syncronized collection can give us ensure in safety. 
In case simple HashMap, behaviour of component can be unpredictable. 

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45588#action_45588 ] 

lhe edited comment on SM-1569 at 9/10/08 3:52 AM:
-------------------------------------------------------------

Applied the patch.
Thanks for pointing this out, Ivan.

One remark:
I am not sure if it is really needed to have the map synchronized as normally dividing by message exchange id should be enough.
Anyway, I put in the code as you posted here.

      was (Author: lhe):
    Applied the patch.
Thanks for pointoing this out, Ivan.

One remark:
I am not sure if it is really needed to have the map synchronized as normally dividing by message exchange id should be enough.
Anyway, I put in the code as you postet here.
  
> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Lars Heinemann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45617#action_45617 ] 

Lars Heinemann commented on SM-1569:
------------------------------------

Ok, seems like a good choice you made ;)
Now things are more clear to me.

Regards
Lars


> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>            Assignee: Lars Heinemann
>             Fix For: 3.3
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SM-1569) CleanUp tempFiles-attachments is not thread-safe

Posted by "Ivan Pryvalov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/SM-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45585#action_45585 ] 

Ivan Pryvalov commented on SM-1569:
-----------------------------------

For id we can use exchange.getExchangeId(). 

> CleanUp tempFiles-attachments is not thread-safe
> ------------------------------------------------
>
>                 Key: SM-1569
>                 URL: https://issues.apache.org/activemq/browse/SM-1569
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-mail
>    Affects Versions: 3.3
>         Environment: SMX-3.3
>            Reporter: Ivan Pryvalov
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.