JIRA

  • Log In Access more options
    • Online Help
    • GreenHopper Help
    • Agile Answers
    • Use Agile By Default
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What’s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
  • Agile Access more options (Alt+g)
  • Create Issue
  • Mule
  • MULE-3302

We still get Message scribbling under certain conditions (Request Response) (It gets caught but we need to remove the underlying probblem)

  • Agile Board
  • More Actions
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Bug Bug
  • Status: Reopened Reopened
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 2.0.1
  • Fix Version/s: Tech. Debt
  • Component/s: Core: Concurrency / Threading
  • Labels:
    • 20-messages
  • User impact:
    Medium
  • Effort points:
    10
  • Similar Issues:
    None

Description

From Dan D.
I was getting slightly annoyed by the build server today so I tracked down a reliable way to reproduce this issue. To reproduce this issue locally just add a Thread.sleep(1000) to VMMessageRequester at line 89 - right after the resetAccessControl().

I think I can even explain the issue here. We have thread #1 which sends off a message which is in essence "new DefaultMuleMessage(new DefaultMuleMessageAdapter());"

Thread #2 (i.e. the main thread in the test case), picks this up via the VMMessageRequester and calls resetAccessControl().

So far so good, except this message has been sent with MuleClient.send() so its going to build a response using the request message's properties. This happens in MuleClient:330:

response = OptimizedRequestContext.unsafeRewriteEvent(response);

This eventually calls RequestContext.newMessage(message) which creates a copy of the message by doing new DefaultMuleMessage(payload, originalMessageAdapter) and resets the MessageAdapter that is also being picked up by the VMMessageRequester. Since it is not deterministic whether this happens after or before the VMMessageRequester gets the message and resets the access control, sometimes it fails and sometimes it doesn't.

Issue Links

relates to

Sub-task - The sub-task of the issue MULE-3300 VMRequestorTestCase fails due to message scribbling

  • Critical - Crashes, loss of data, severe memory leak.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. MULE-893 Clarify/reachitect MuleMessage property mutability & handling

  • Critical - Crashes, loss of data, severe memory leak.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. MULE-1564 VMLoanBrokerSynchronousFunctionalTestCase logs a warning regarding a null MULE_REMOTE_SYNC property

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.
  • Options
    • Show All
    • Show Open

Sub-Tasks

1.
VMRequestorTestCase fails due to message scribbling Sub-task Closed Closed Pablo Kraan
 

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
  • Transitions
  • Commits
  • Source
  • Builds
Hide
Permalink
Ross Mason added a comment - 05/May/08 11:36 AM

good synposis. maybe we should disable send() when using VM queues since you're not going to get a response from the queue anyway.

I did some further digging around the message scribbling and found the following -

1. Message rewriting has the biggest impact on performance (yourkit shows that way too much time is spent writing property maps)
2. we use RequestContext.safeRewriteEvent for writing response message properties when performing req/res messaging
3. We often use RequestContext.unsafeRewriteEvent as a safe-guard in case the message changed
4. When using send() we often return the original message when there is no result from the send operation.

All of this is throwback from Mule 1.x. We have fixed the route causes for doing these kinds of things in Mule 2.x except for one (request/response handling)

WRT Request Response handing (point 2), right now we 'merge' request and response messages which is confusing at best. The reason this is done is because you some times need properties on the request message to be available after you get a response back. I think there are two ways of handling this -

  • Have a specialisation of MuleMessage called RequestResponseMuleMessage that maintains a reference to both the request and the response message and provides interfaces for accessing the request message. This means people will have to check for the message type and cast it, or we make a bigger change to the send() method signature
  • Ignore the request message but provide a way let users configure which properties they want to be carried over from the request message to the response message. This could be done using a PropertyScope i.e. Session could be used, but not ideal. Or we could add a new scope for this purpose.

Point 3 should not be necessary any longer since Message payloads are not immutable as they were in Mule 1.x. The rewrite action is the most expensive. I believe we should only have to create a DefaultMuleMessage once for each request. This would improve performance.

Point 4. We would need to wrap a 'null' return into a NullPayload when using MuleClient.send() since we never return a null right now. This is partly be cause if anything does go wrong we can attach an exception payload to the return message. I think it would be less disruptive if we kept this behaviour.

IMO one of the biggest enabler of message scribbling is the VM transport because it allows the mule to "copy" the message over two threads. The other is our handling of request/response message flows and sync message flow in VM (which Dan points out).

Show
Ross Mason added a comment - 05/May/08 11:36 AM good synposis. maybe we should disable send() when using VM queues since you're not going to get a response from the queue anyway. I did some further digging around the message scribbling and found the following - 1. Message rewriting has the biggest impact on performance (yourkit shows that way too much time is spent writing property maps) 2. we use RequestContext.safeRewriteEvent for writing response message properties when performing req/res messaging 3. We often use RequestContext.unsafeRewriteEvent as a safe-guard in case the message changed 4. When using send() we often return the original message when there is no result from the send operation. All of this is throwback from Mule 1.x. We have fixed the route causes for doing these kinds of things in Mule 2.x except for one (request/response handling) WRT Request Response handing (point 2), right now we 'merge' request and response messages which is confusing at best. The reason this is done is because you some times need properties on the request message to be available after you get a response back. I think there are two ways of handling this -
  • Have a specialisation of MuleMessage called RequestResponseMuleMessage that maintains a reference to both the request and the response message and provides interfaces for accessing the request message. This means people will have to check for the message type and cast it, or we make a bigger change to the send() method signature
  • Ignore the request message but provide a way let users configure which properties they want to be carried over from the request message to the response message. This could be done using a PropertyScope i.e. Session could be used, but not ideal. Or we could add a new scope for this purpose.
Point 3 should not be necessary any longer since Message payloads are not immutable as they were in Mule 1.x. The rewrite action is the most expensive. I believe we should only have to create a DefaultMuleMessage once for each request. This would improve performance. Point 4. We would need to wrap a 'null' return into a NullPayload when using MuleClient.send() since we never return a null right now. This is partly be cause if anything does go wrong we can attach an exception payload to the return message. I think it would be less disruptive if we kept this behaviour. IMO one of the biggest enabler of message scribbling is the VM transport because it allows the mule to "copy" the message over two threads. The other is our handling of request/response message flows and sync message flow in VM (which Dan points out).
Hide
Permalink
Ross Mason added a comment - 05/May/08 11:37 AM

From Dan D.

> good synposis. maybe we should disable send() when using VM queues since you're not going to get a response from the queue anyway.

I think this makes a lot of sense...

> - Have a specialisation of MuleMessage called RequestResponseMuleMessage that maintains a reference to both the request and the response message and > provides interfaces for accessing the request message. This means people will have to check for the message type and cast it, or we make a bigger change to > the send() method signature

That seems kind of ugly to me at first glance. What about introducing the concept of an Exchange object which has references to the request/response messages. so you could do message.getExchange().getRequest() or getInMessage()

Show
Ross Mason added a comment - 05/May/08 11:37 AM From Dan D. > good synposis. maybe we should disable send() when using VM queues since you're not going to get a response from the queue anyway. I think this makes a lot of sense... > - Have a specialisation of MuleMessage called RequestResponseMuleMessage that maintains a reference to both the request and the response message and > provides interfaces for accessing the request message. This means people will have to check for the message type and cast it, or we make a bigger change to > the send() method signature That seems kind of ugly to me at first glance. What about introducing the concept of an Exchange object which has references to the request/response messages. so you could do message.getExchange().getRequest() or getInMessage()
Hide
Permalink
Ross Mason added a comment - 05/May/08 11:38 AM

From Andrew P.

>- Ignore the request message but provide a way let users configure which properties they want to be carried over from the request message to the response >message. This could be done using a PropertyScope i.e. Session could be used, but not ideal. Or we could add a new scope for this purpose.

New scope makes sense, which would be for the duration of the request. Quote often it's called 'flash' scope these days, when a single response still has access to the request and then the ref is automatically discarded (unlike e.g. session scope, which assumes the developer knowing when to drop the data).

Show
Ross Mason added a comment - 05/May/08 11:38 AM From Andrew P. >- Ignore the request message but provide a way let users configure which properties they want to be carried over from the request message to the response >message. This could be done using a PropertyScope i.e. Session could be used, but not ideal. Or we could add a new scope for this purpose. New scope makes sense, which would be for the duration of the request. Quote often it's called 'flash' scope these days, when a single response still has access to the request and then the ref is automatically discarded (unlike e.g. session scope, which assumes the developer knowing when to drop the data).
Hide
Permalink
Ross Mason added a comment - 05/May/08 11:42 AM

The other thing I think should be addressed is clearing the RequestContext once there is no request in progress. Right now there is an event associated with a thread even after the event has been processed as long as a new event is not available for the thread.
Once we are in the Messge Dispatcher (or returning to a callee in a MessageReceiver) we should clear the RequestContext. This would be cleaner and maintain the system in a consistent state.

Show
Ross Mason added a comment - 05/May/08 11:42 AM The other thing I think should be addressed is clearing the RequestContext once there is no request in progress. Right now there is an event associated with a thread even after the event has been processed as long as a new event is not available for the thread. Once we are in the Messge Dispatcher (or returning to a callee in a MessageReceiver) we should clear the RequestContext. This would be cleaner and maintain the system in a consistent state.
Ross Mason made changes - 05/May/08 11:42 AM
Field Original Value New Value
Priority To be reviewed [ 6 ] Major [ 3 ]
Dirk Olmes made changes - 06/May/08 02:07 AM
Link This issue relates to MULE-3300 [ MULE-3300 ]
Dirk Olmes made changes - 06/May/08 02:08 AM
Link This issue relates to MULE-893 [ MULE-893 ]
Dirk Olmes made changes - 06/May/08 02:09 AM
Link This issue relates to MULE-1564 [ MULE-1564 ]
Andrew Perepelytsya made changes - 06/May/08 11:08 AM
Workflow Copy of Main Mule Workflow [ 59631 ] Fixed Main Mule Workflow (after JIRA upgrade) [ 59649 ]
Daniel Feist made changes - 12/May/08 07:28 PM
Fix Version/s 2.0.x Backlog [ 10352 ]
Andrew Perepelytsya made changes - 15/May/08 10:19 AM
Workflow Fixed Main Mule Workflow (after JIRA upgrade) [ 59649 ] Copy of Main Mule Workflow [ 66446 ]
Andrew Perepelytsya made changes - 15/May/08 10:55 AM
Workflow Copy of Main Mule Workflow [ 66446 ] Fixed Main Mule Workflow (after JIRA upgrade) [ 70087 ]
Travis Carlson made changes - 11/Jun/08 12:55 PM
Priority Major [ 3 ] Critical [ 2 ]
Daniel Feist made changes - 03/Jul/08 02:10 PM
Labels 20-messages
Hide
Permalink
Daniel Feist added a comment - 03/Jul/08 02:11 PM

Is there anything here that we should do before 2.0.2?

Show
Daniel Feist added a comment - 03/Jul/08 02:11 PM Is there anything here that we should do before 2.0.2?
Daniel Feist made changes - 03/Jul/08 02:11 PM
Assignee Ross Mason [ ross ]
Daniel Feist made changes - 10/Jul/08 06:29 AM
Effort points 10
Hide
Permalink
Ross Mason added a comment - 03/Sep/08 11:18 PM

It would be good to fix this, but I don't think its a priority for 2 EE right now. Keeping critical though

Show
Ross Mason added a comment - 03/Sep/08 11:18 PM It would be good to fix this, but I don't think its a priority for 2 EE right now. Keeping critical though
Ross Mason made changes - 03/Sep/08 11:18 PM
Resolution Fixed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]
Hide
Permalink
Daniel Feist added a comment - 04/Sep/08 05:21 PM

Did you mean to close this Ross or just reduce priority?

Show
Daniel Feist added a comment - 04/Sep/08 05:21 PM Did you mean to close this Ross or just reduce priority?
Hide
Permalink
Ross Mason added a comment - 04/Sep/08 07:17 PM

oops, didn't mean to close this

Show
Ross Mason added a comment - 04/Sep/08 07:17 PM oops, didn't mean to close this
Ross Mason made changes - 04/Sep/08 07:17 PM
Resolution Fixed [ 1 ]
Status Closed [ 6 ] Reopened [ 4 ]
Ross Mason made changes - 04/Sep/08 07:17 PM
Priority Critical [ 2 ] Major [ 3 ]
Daniel Feist made changes - 11/Sep/08 01:53 PM
Fix Version/s 2.x Backlog [ 10440 ]
Fix Version/s 2.0.x Backlog [ 10352 ]
Daniel Feist made changes - 12/Nov/08 08:58 PM
Fix Version/s Tech. Debt [ 10572 ]
Fix Version/s 2.x Backlog [ 10440 ]
Daniel Feist made changes - 28/Nov/08 07:27 AM
Labels 20-messages 20-messages 22-techdebt-candidates
Daniel Feist made changes - 12/Dec/08 09:23 AM
Labels 20-messages 22-techdebt-candidates 20-messages
Ross Mason made changes - 10/May/09 05:45 AM
Assignee Ross Mason [ ross ]
Daniel Feist made changes - 22/Dec/09 04:36 AM
Link This issue is blocked by BL-13 [ BL-13 ]
Transition Time In Source Status Execution Times Last Executer Last Execution Date
Open Open Closed Closed
121d 11h 42m 1 Ross Mason 03/Sep/08 11:18 PM
Closed Closed Reopened Reopened
19h 59m 1 Ross Mason 04/Sep/08 07:17 PM
This list may be incomplete, as errors occurred whilst retrieving source from linked applications:
  • Repository mule on http://foo.bar/ failed: Error in remote call to 'FishEye 0 (http://foo.bar/)' (http://foo.bar) [AbstractRestCommand{path='/rest-service-fe/changeset-v1/listChangesets/', params={expand=changesets[-21:-1].revisions[0:29], comment=MULE-3302, p4JobFixed=MULE-3302, rep=mule}, methodType=GET}] : java.net.UnknownHostException: foo.bar

People

  • Assignee:
    Unassigned
    Reporter:
    Ross Mason
Vote (0)
Watch (0)

Dates

  • Created:
    05/May/08 11:36 AM
    Updated:
    22/Dec/09 04:36 AM

Agile

  • View on Board
  • Atlassian JIRA (v5.0.7#734-sha1:8ad78a6)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for MuleForge. Try JIRA - bug tracking software for your team.