Snitz Forums 2000 - Bad and Sloppy Coding.

Snitz Forums 2000

Username:	Password:
Save Password
Forgot your Password?

All Forums

Snitz Forums 2000 MOD-Group

MOD Add-On Forum (W/Code)

Bad and Sloppy Coding.

New Topic

Topic Locked

Printer Friendly

Author

Topic

Page: of 3

Hopper
Junior Member

192 Posts

Posted - 27 December 2000 : 22:20:24

Ok people.. Im putting this here because its a serious matter to do with the coding of mods that ive seen. People are getting extreamly sloppy about closing the Connections they are creating to the databases. This leaves active connections going and in time will crash the servers. I know this because its happening on my host. When it first happend I started going through some of the Mod code and I was amazed at how people leave their connections open. Active Users is a wonderful example. Every connection that is created is not closed after its finished being used so every tmie someone accesses these files they are leaving active connections. New connections are created every time the files are accessed but none are closed. Anyone using this mod should either remove it or fix it yourself. I suggest that if anyone is going to put Mods in they go through the code and make sure these types of things are done. And Mod creators, please make sure your code is clean. This is causing an enormous problem.

frankie
Junior Member

Vatican City
304 Posts

Posted - 27 December 2000 : 22:30:41

Hop,

I also noticed that..and I did close the objConn in my inc_top page. Are there any other connections we need to close in that mod (active_users)?!!

Note: Do we have to close the recordset objects too??!

ô¿~

Edited by - frankie on 27 December 2000 22:31:45

Hopper
Junior Member

192 Posts

Posted - 27 December 2000 : 22:34:48

<BLOCKQUOTE id=quote>quote:<hr height=1 noshade id=quote>
Hop,

I also noticed that..and I did close the objConn in my inc_top page. Are there any other connections we need to close in that mod (active_users)?!!

Note: Do we have to close the recordset objects too??!

ô¿~

Edited by - frankie on 27 December 2000 22:31:45
<hr height=1 noshade id=quote></BLOCKQUOTE id=quote>

Just for the sake of clean code you should close anything you open. This avoids problems. There are a huge number of places im finding that connections arnt being closed. What im doing is getting a list of ALL files in the forum directory where the string "Server.CreateObject" is located and following the code through to see if the connections are closed. You'd be surprised how many open connections are left open.. *sigh* its also a nightmare to clean up.

Edited by - Hopper on 27 December 2000 22:35:29

Da_Stimulator
DEV Team Forum Moderator

USA
3373 Posts

Posted - 27 December 2000 : 22:36:32

Always a good Idea to close all connections, and objects.

When ever I set an object, before I even start writing code in it, I close it.

For instance, I'll put set rs = my_conn.execute(something)

Then the next line I'll put set rs = nothing

before I do anything. Since I'm hosted on CFM Resources, I dont need any open connections to help an already clumbsy server.

Da_Stimulator
Stimmy's Forums - Using 3.1 SR3
"We all die someday, but your goin down before I do" - Me

Da_Stimulator
DEV Team Forum Moderator

USA
3373 Posts

Posted - 27 December 2000 : 23:23:03

Heres the track record of the stuffs I changed :)

view_announcments.asp (Richards)
Added at line 62<pre id=code> set rs = nothing
my_conn.close
set my_conn = nothing
Response.Redirect(strForumURL)</pre id=code>

Added at line 155 <pre id=code> set rs = nothing</pre id=code>

Topic.asp
Added at line 110 <pre id=code> set rsCStatus = nothing
set rsFStatus = nothing
set rsTStatus = nothing</pre id=code>
Added at line 787 <pre id=code>set rsCStatus = nothing
set rsFStatus = nothing
set rsTStatus = nothing
set rsCat = nothing
set rsCount = nothing
set rs = nothing</pre id=code>

setup_login.asp
Added at line 85<pre id=code>set dbRs = nothing
my_conn.close
set my_conn = nothing</pre id=code>

Added at the end of the file
<pre id=code><%
set dbRs = nothing
my_conn.close
set my_conn = nothing
%></pre id=code>

setup.asp
NOTHING, whoever wrote setup.asp keeps track of there stuff :)

search.asp
Added after 'set rs = nothing' <pre id=code> set rsCFStatus = nothing</pre id=code>

privatesent.asp
Added after line 158 <pre id=code>set rsMessage = nothing
set rs = nothing</pre id=code>

privatesend.asp
Line 304<pre id=code>set rs = nothing
set rsName = nothing
set rsReply = nothing
set rsTopicAuthor = nothing</pre id=code>

privateread.asp
line 190<pre id=code>set rs = nothing
set rsMessage = nothing</pre id=code>

post_info.asp - Clean

post.asp - Clean

pop_mail.asp - Clean

pop_icon_legend.asp - Almost Clean (smiles mod installed)
Fixed set carRS1=nothing to set catRS1=nothing
SMall error on my part

pop_delete.asp - Clean

pm_view.asp
added after 'loop' on line 157<pre id=code> set rsMessage = nothing</pre id=code>
added before 'rsMessage.MoveNext' line 156 <pre id=code>set rsFrom = nothing</pre id=code>
line 266<pre id=code><% set RSto = nothing
rsFMessage.MoveNext
Loop
set rsFMessage = nothing</pre id=code>

To be continued.....

Da_Stimulator
Stimmy's Forums - Using 3.1 SR3
"We all die someday, but your goin down before I do" - Me

frankie
Junior Member

Vatican City
304 Posts

Posted - 28 December 2000 : 00:47:56

If you guys looking for a Support moderator,

Da_Stimulator is the man, (he's far from being an average member).

Note: Stimmy, you'll get my vote in case there's no recount! <img src=icon_smile_big.gif border=0 align=middle>

ô¿~

Edited by - frankie on 28 December 2000 00:49:58

HuwR
Forum Admin

United Kingdom
20584 Posts

Posted - 28 December 2000 : 05:45:35

Hopper,

Firstly, you are misleading people with your statement, while I agree it is good practice too close DB connections, it is NOT necesary.By default, a connection terminates after your script finishes execution.

Having too many open connections should not make your server crash. I would affect performance, and cause connection timeouts for people trying to connect, but should not crash your server.

Your statement about the active users is also not correct, active_users uses a single db connection,my_conn, the connection object which is instantiated in inc_top and closed in inc_footer.

It does not open any other connections, the recordsets are all related to the same connection.

If your host is experiencing server crashes due to connection overload, then it is more likely because they are trying to cram too many web sites onto the same server with inadequate resources, than an unclosed ADO connection.

'Resistance is futile'

davemaxwell
Access 2000 Support Moderator

USA
3020 Posts

Posted - 28 December 2000 : 08:27:45

HuwR is right. We should be using only ONE connection throughout the whole process. I don't know why we're opening other connections because they are not needed. All mods I create use the original connection opened in inc_top and closed in inc_footer. There should be no other connections made.....

The more connections/disconnects there are, the slower the forum becomes. Sharing one connection over the life of the page is much faster.....

Dave Maxwell
--------------
When's the next meeting of Snitzaholics Anonymous<img src=icon_smile_question.gif border=0 align=middle>

Hopper
Junior Member

192 Posts

Posted - 28 December 2000 : 11:48:29

I was talking about within Mods. People ARE opening new connections and they are not closing them. As for the comment about it crashing the server.. this is just what the Host told me.. Dont shoot them messanger. Appologies if I came across like the base forum code was this way.

davemaxwell
Access 2000 Support Moderator

USA
3020 Posts

Posted - 28 December 2000 : 13:33:30

<BLOCKQUOTE id=quote>quote:<hr height=1 noshade id=quote>
People ARE opening new connections and they are not closing them.
<hr height=1 noshade id=quote></BLOCKQUOTE id=quote>
OK, I just want to be sure we're talking about the same thing here. Here is what I consider a connection opening:

set my_Conn = Server.CreateObject("ADODB.Connection")
my_Conn.Open strConnString

I DON'T consider this a new connection:
set rs = my_Conn.Execute (strSql)

So when this statement is executed, set rs = nothing, I consider that closing the cursor, not closing a connection. my_conn.close and Set my_Conn = nothing is what actually closes the connection. Are we using the same terminology??

The set rs=nothing is good programming practice and in the end will speed up things, but there is no server degradation if they are not closed because they automatically get closed when the connection is closed. Not a big deal, but doing the closes of the cursors will speed up processing.

Actually opening and closing connections WILL slow up the page (and the server obviously), so this should NOT be done and the connection should always be shared.....
<BLOCKQUOTE id=quote>quote:<hr height=1 noshade id=quote>
As for the comment about it crashing the server.. this is just what the Host told me.. Dont shoot them messanger. Appologies if I came across like the base forum code was this way.
<hr height=1 noshade id=quote></BLOCKQUOTE id=quote>
We're not trying to shoot the messenger, but Snitz has been blamed more than a few time for server problems, but a majority of the time the problem is with the way IIS was installed/upgraded. I'm sure you could find a number of posts to back up this statement (Sorry, too lazy to search for them myself right now...)

Dave Maxwell
--------------
When's the next meeting of Snitzaholics Anonymous<img src=icon_smile_question.gif border=0 align=middle>

Hopper
Junior Member

192 Posts

Posted - 28 December 2000 : 14:26:19

Yes, we are on the same page.

set my_Conn = Server.CreateObject("ADODB.Connection")
my_Conn.Open strConnString

The Mods i am talking about change the my_Conn to something else to make their own connection but the syntax is the same. There are no Conn.close/set Conn = nothing to clean up those connections.

HuwR
Forum Admin

United Kingdom
20584 Posts

Posted - 28 December 2000 : 15:16:19

So which mods are we talking about, the active_users that I have does NOT do this.

'Resistance is futile'

Da_Stimulator
DEV Team Forum Moderator

USA
3373 Posts

Posted - 28 December 2000 : 15:46:13

<BLOCKQUOTE id=quote>quote:<hr height=1 noshade id=quote>
If you guys looking for a Support moderator,

Da_Stimulator is the man, (he's far from being an average member).

Note: Stimmy, you'll get my vote in case there's no recount! <img src=icon_smile_big.gif border=0 align=middle>

ô¿~

Edited by - frankie on 28 December 2000 00:49:58
<hr height=1 noshade id=quote></BLOCKQUOTE id=quote>

Ummm thx :) but I dont think I know enough about all the code yet to be a support moderator.
Besides, I'm still getting support from Huwr and Richard, Dave and everyone, Wouldnt make sense, but thx for the thought :)

Da_Stimulator
Stimmy's Forums - Using 3.1 SR3
"We all die someday, but your goin down before I do" - Me

frankie
Junior Member

Vatican City
304 Posts

Posted - 28 December 2000 : 16:29:40

Stimmy,

All I meant is that I needed you up there with the masters for your hard work and dedication to this valuable forum(e.g. Richard, HuwR, etc...)

ô¿~

Podge
Support Moderator

Ireland
3775 Posts

Posted - 28 December 2000 : 19:46:44

Ok, I don't want to fan the flames of this rather heated argument but....

I don't think that the sloppy coding is just isolated to not closing connections but also to the likes of this in pop_delete.asp (latest version lines 504-508

<% end if %>
<% end if %>
<% end if %>
<% end if %>
<% end if %>

I know that it's been mentioned before and its disappearing slowly from the code but I think that if we concentrated a bit on conventions, good code practices etc., it would pay off big time in the long run.

Podge
-----------------------
www.thehungersite.com
Click to donate free food.

slemieux
Junior Member

USA
234 Posts

Posted - 29 December 2000 : 07:43:15

I think just about everyone knows that closing your connections is always a good idea, but since Windows will close them for you, it can be an easy thing to forget about. It shouldn't be crashing your server!

Some techs have a tendency to tell what THEY want to here, even if it make no sense. I had one host who (stayed with them for 6 days) told me it was impossible to run .cgi on windows, you could run .pl but not .cgi. Argued with 6 tech guys finally found one that would listen and I walked him through mapping .cgi on the server. Took him 5 minutes.

I won't mention any names but it sounds alot like halfassedhosting.com. Remember if your getting "half price hosting", your getting what you pay for!

Scott LeMieux
Ready... Fire... Aim!! <img src="http://www.thepercussionist.com/adf/dance2.gif" border=0>

Page: of 3

Topic

New Topic

Topic Locked

Printer Friendly

Jump To:

Snitz Forums 2000

This page was generated in 0.15 seconds.