aMule Bug Tracker - aMule
View Issue Details
0000439aMuleMiscpublic2005-05-18 11:242005-05-20 02:03
eyalzo 
Kry 
normalcrashrandom
resolvedfixed 
2.0.0 
SVN 
0000439: Null pointers in ListenSocket.cpp
Under very heavy load, amule crashes because of null pointer . It's a matter of luck, because there is no resource-lock there, so checking a pointer just before using it just improve the chances not to crash.
I saw that one of these crahses in ListenSocket.cpp was already fixed in 2.0.0final, but there is one more:

Before:
~~~~~~~~

case OP_AICHFILEHASHREQ: {
if (m_client->IsSupportingAICH() && reqfile->GetAICHHashset()->GetStatus() == AICH_HASHSETCOMPLETE
                                && reqfile->GetAICHHashset()->HasValidMasterHash())


After:
~~~~~~~
case OP_AICHFILEHASHREQ: {
if (m_client && reqfile && m_client->IsSupportingAICH() && reqfile->GetAICHHashset()->GetStatus() == AICH_HASHSETCOMPLETE
                                && reqfile->GetAICHHashset()->HasValidMasterHash())
No tags attached.
Issue History
2005-05-18 11:24eyalzoNew Issue
2005-05-18 11:29eyalzoNote Added: 0000977
2005-05-19 01:14KryStatusnew => assigned
2005-05-19 01:14KryAssigned To => Kry
2005-05-19 01:14KryNote Added: 0000979
2005-05-19 01:14KryStatusassigned => acknowledged
2005-05-19 02:03KryNote Added: 0000981
2005-05-19 02:06KryNote Edited: 0000981
2005-05-19 07:10eyalzoNote Added: 0000982
2005-05-20 02:03KryStatusacknowledged => resolved
2005-05-20 02:03KryFixed in Version => CVS
2005-05-20 02:03KryResolutionopen => fixed
2005-05-20 02:03KryNote Added: 0000985

Notes
(0000977)
eyalzo   
2005-05-18 11:29   
There is one more in:

                if (m_client->IsSupportingAICH() && pPartFile->GetAICHHashset()->GetStatus() == AICH_HASHSETCOMPLETE

Which I have changed to:
                if (m_client && m_client->IsSupportingAICH() && pPartFile->GetAICHHashset()->GetStatus() == AICH_HASHSETCOMPLETE
(0000979)
Kry   
2005-05-19 01:14   
ACK (you're good, btw, many thanks)
(0000981)
Kry   
2005-05-19 02:03   
(edited on: 2005-05-19 02:06)
Actually, your tests are not needed. The events are sync (so i.e. one client can NEVER be deleted from another event/thread). The places where those tests are added are strategically placed: one client can only die on a SendPacket function.

So this crashes should _never_ happen. If they do, there's a bigger problem out there. I could have skipped some SendPacket call, so feel free to correct me.

edited on: 05-19-05 02:06
(0000982)
eyalzo   
2005-05-19 07:10   
Thanks. I didn't know that. I just had several crashes in rc8, so I added these tests everywhere, and problem was solved. Now I see that these two were not the reason, but the other two that were already fixed in 2.0.0final.
And, I think it's a good practice to add tests like these even when they're not needed. Becuase one day someone might add a SendPacket somewhere...
(0000985)
Kry   
2005-05-20 02:03   
Well, what's life without new bugs ;) Closing anyway :)