[YAMos-dev] Re: Preventing use of invalid pointers

  • From: Christer Oldhoff <coldhoff@xxxxxxxxxx>
  • To: yamos-dev@xxxxxxxxxxxxx
  • Date: Thu, 23 Nov 2006 12:29:38 +0100

Hello Jens,

On 2006-11-23, Jens Langner wrote:

> Hello Christer,
> On Wed, 22 Nov 2006 23:12:42 +0100, Christer Oldhoff wrote:
> [...]
>> As a consequence of the facts 1)-3), my suggestion to manipulate
>> CleanupReadMailData() by setting rmData->mail=NULL before it is called
>> was not a good idea, nor is it a solution to call DoMethod(...
>> rmData->readWindow,...MUIA_Window_CloseRequest, TRUE) followed by
>> free(mail) from RemoveMailFromList(), because one cannot guarantee that
>> rmData->mail has not become an invalid pointer by the time
>> CleanupReadMailData() executes.
> Sorry, but I am again a bit confused. So what is your conclusion now? Can
> you verify that the problem is finally gone now or not?
No, sorry, the problem is still there.
Every attempt to fix it has either broken something else or not cured all 
problems or both.

The fundamental problem is that free(mail) is called to early when a mail which 
is displayed in a read window is removed from a folder.

Let me give You a summary of the entire invalid pointer situation.

There are two somewhat different cases causing trouble:

A) The one in which the read window closes automatically because of some user 
interaction with the read window (bug #140858).

B) The one in which the read window does *not* close although the mail it shows 
has been removed (by user interaction with main window, or filter ops, or 

We tried this:

In case A, the premature free(mail) means that CleanupReadMailData() is called 
with an invalid rmData->mail pointer.
This caused the original bug #140858 crashes.
My initial suggestion to set rmData->mail = NULL prevents the crashes, but also 
stops CleanupReadMailData() from cleaning up all temporary files.

From this I derived the conditions 1) and 3) which any proper fix for this bug 
must satisfy (if the cleanup code is not changed completely).

Thore discovered case B.
It leaves the read window with an invalid rmData->mail pointer.

In an attempt to fix this, code was added to RemoveMailFromList() which checks 
through the global rmData list, and then calls 
DoMethod(... rmData->readWindow,...MUIA_Window_CloseRequest, TRUE)
if it finds an open read window with the mail that is about to be removed.

This second fix fails because of fact 2); CleanupReadMailData() is again called 
with an invalid rmData->mail pointer.
Of course, if the requirements 1) and 3) were met, this way of making the read 
window close would work, but it has other issues as You have pointed out.

CleanupReadMailData() does its job well. You guys must make sure it is always 
called with a valid rmData->mail pointer.
You also need a satisfactory way to let a read window know it has to close in 
case B.

Your suggestion with deletion/removal flags might be the solution, but I'll 
better stop proposing fixes, because I cannot compile YAM to test them myself
before sending them to you developers.
Instead, I'll concentrate on analyzing (= try to find problems with) fixes You 
guys make, to learn and to help ;-)  

I hope this was clearer :)

Christer Oldhoff
Solna, Sweden

Email: coldhoff@xxxxxxxxxx
YAM developer mailing list - http://www.freelists.org/list/yamos-dev
Listserver help...: mailto:yamos-dev-request@xxxxxxxxxxxxx?subject=HELP
Unsubscribe: mailto:yamos-dev-request@xxxxxxxxxxxxx?subject=UNSUBSCRIBE

Other related posts: