Well Ian did a stealth change awhile back. static int socketValid(SocketPtr s) { if (s && s->privateSocketPtr && thisNetSession && (s->sessionID == thisNetSession)) return true; interpreterProxy->success(false); return false; }
Note the addition of && thisNetSession check which fixes the problem. versus the older code which was:
static int socketValid(SocketPtr s) { if ((s != 0) && (s->privateSocketPtr != 0) && (s->sessionID == thisNetSession)) return true; interpreterProxy->success(false); return false; }
This is the WIndows code (I think) / ************************************************************************ ***** SocketValid: Validate a given SocketPtr ************************************************************************ *****/ static int SocketValid(SocketPtr s) { if ((s != NULL) && (s->privateSocketPtr != NULL) && (s->sessionID == thisNetSession)) { return true; } else { FAIL(); return false; } }
Someone (I sure there is a windows user out there somewhere?) could try the suggested failure case (attached below) and see if this problem happens on the windows side? Since the copy of the windows code I have looks like the old faulty Unix code.
On May 11, 2004, at 12:24 PM, tim@sumeru.stanford.edu wrote:
It looks like no one but John has yet made any change relating to this problem; should we? It looks a simple enough change but I'm vague enough about sockets to not know whether it is sufficient.
Original note
This change prevents the following failure case where you can create a socket before initializing the network which then causes the VM to core dump after a save/restart when it attempts to destroy the socket say as a result of weak reference cleanup.
This won't work on the macintosh VMs because another check fails the socket creation because OpenTransport isn't initialized yet, and I did roll the fix into the macintosh vm 3.5.1b4 because it uses the unix socket code now. But you alert Unix/Linux users might want to try this and see what happens.
Anyone know what Windows does?
| socket | socket _ Socket newTCP. socket listenOn: 44444 backlogSize: 4. ^socket
this gives you
semaphore: a Semaphore() socketHandle: a ByteArray(0 0 0 0 0 0 0 0 2 96 51 16) /*Note how session id is ZERO */ readSemaphore: a Semaphore() writeSemaphore: a Semaphore() primitiveOnlySupportsOneSemaphore: false
Now save the image, restart, then issue a destroy on that Socket and BOOM {I'll note sometimes I don't get a crash but the debugging malloc code complains about an invalid free, or double free since the privateptr is bogus but read/writeable...}
I got this because KomServices does this. TcpListener>>newListener: backlogSize "Create a new socket that listens on our port. The backlog is how many simultaneous connections to accept at the same time"
[^self pvtNewListener: backlogSize] on: Error do: [].
"Try one more time after initializing the network" Socket initializeNetwork. ^self pvtNewListener: backlogSize.
TcpListener>> pvtNewListener: backlogSize "Create a new socket that listens on our port. The backlog is how many simultaneous connections to accept at the same time"
| listener | listener := self socketClass newTCP. self socketsToDestroy add: listener. listener listenOn: portNumber backlogSize: backlogSize. ^listener
I have to wonder if an accept can occur and we can get a socket created with a zero session id too?
New unix code should look like below. Can't speak for what the windows code should look like, but the key is that thisNetSession should NOT be zero when we run this check.
/* answer whether the given socket is valid in this net session */
static int socketValid(SocketPtr s) { if ((s != 0) && (s->privateSocketPtr != 0) && (s->sessionID == thisNetSession) && (thisNetSession != 0)) return true; interpreterProxy->success(false); return false; }
Will need someone to approve and make the unix code changes... -
-- ======================================================================== === John M. McIntosh johnmci@smalltalkconsulting.com 1-800-477-2659 Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com ======================================================================== ===