Fork me on GitHub
Subscribe 4

Ticket #1041 (fixed bug)

Fatal error mysqli_free_result

  • Created: 2015-06-20 10:09:15
  • Reported by: Otomatic
  • Assigned to: quy
  • Milestone: 1.5.9
  • Component: database
  • Priority: normal

According to the PHP and MySQL versions used, one can get the following error:
: mysqli_free_result() expects parameter 1 to be mysqli_result, boolean given in forum/include/dblayer/mysqli.php on line 186
To avoid this error, just change the function close() in forum\include\dblayer\mysqli.php
replace

if ($this->query_result)				@mysqli_free_result($this->query_result);

by

if (!is_bool($this->query_result))				@mysqli_free_result($this->query_result);

We must also change other files related to Mysql

History

adaur 2015-06-20 11:16:24

  • Milestone set to 1.5.9.

Can you give us the PHP and MySQL versions that give the error please?

Otomatic 2015-06-20 13:14:21

It is on the alpha test of new PHP version for the hoster Free.fr
PHP 5.6.8 edited by Free with some functions disabled.
MySQL :
Client API version mysqlnd 5.0.11-dev - 20120503
With exactly the same version and the same files of FluxBB, I can not reproduce the problem with:
- Local Apache 2.4.12, PHP5.6.10, MySQL 5.6.25
- Gandi Apache 2.4.12, PHP5.4.41, MySQL 5.5.43
I think the problem is due to the version of MySQL at Free.
However, as the last result of a MySQL query can be a Boolean, eg UPDATE, I think is necessary to make the change.

Comment edited 1 times (Diff)

adaur 2015-06-20 13:57:09

We should apply your fix if it doesn't break current installations. I'll try to investigate later.

chris98 2015-06-21 07:23:00

PHP 5.6.8 edited by Free with some functions disabled.

What functions are disabled?

Otomatic 2015-06-21 07:56:22

The list of disabled functions is provisional because it is an alpha test.
chown, chmod, get_current_user, php_uname, putenv, set_time_limit, getmyuid, getmypid, dl, ini_alter, ini_restore, ini_set, exec, passthru, system, popen, pclose, leak, mysql_list_dbs, listen, chgrp, disk_total_space, disk_free_space, rmdir, realpath, tmpfile, link, shell_exec, proc_open, chroot, openlog, closelog, syslog, flock, socket_create_listen, socket_accept, socket_listen, sleep, usleep, umask, set_include_path, restore_include_path, symlink, setlocale, imagerotate

chris98 2015-06-21 07:59:34

I don't see anything in there that should be affecting it. I think there must be something else to blame.

Comment edited 1 times (Diff)

chris98 2015-06-21 08:03:07

I couldn't find anything regarding mysqli_free_result, but its predecessor, mysql_free_result said this on the manual:

If a non-resource is used for the result, an error of level E_WARNING will be emitted. It's worth noting that mysql_query() only returns a resource for SELECT, SHOW, EXPLAIN, and DESCRIBE queries.

Source: http://php.net/manual/en/function.mysql-free-result.php

Where do you get this error?

Comment edited 1 times (Diff)

Otomatic 2015-06-21 10:17:26

I remove the modification.
http://air.intakes.free.fr/forum/index.php
then any category and any discussion, for example "Les Promotions" then promo 63/66
Fatal error: mysqli_free_result() expects parameter 1 to be mysqli_result, boolean given in /var/www/sdb/d/3/air.intakes/forum/include/dblayer/mysqli.php on line 186

chris98 2015-06-21 10:29:56

Try to find out what Boolean it actually is. It looks like it's happening in $db->close() - see this code:

	function close()
	{
		if ($this->link_id)
		{
			if ($this->query_result)
				@mysqli_free_result($this->query_result);

			return @mysqli_close($this->link_id);
		}
		else
			return false;
	}

See if you can find out what Boolean it actually is. Temporarily, replace that function with this:

	function close()
	{
		if ($this->link_id)
		{
			if ($this->query_result)
			{
				if ($this->query_result === false)
					echo 'This is false';
				else
					echo 'This is true';
				
				//@mysqli_free_result($this->query_result);
			}

			return @mysqli_close($this->link_id);
		}
		else
			return false;
	}

I'm guessing it's due to this:

The PHP Manual wrote:

For SELECT, SHOW, DESCRIBE, EXPLAIN and other statements returning resultset, mysql_query() returns a resource on success, or FALSE on error.

For other type of SQL statements, INSERT, UPDATE, DELETE, DROP, etc, mysql_query() returns TRUE on success or FALSE on error.

mysql_query() will also fail and return FALSE if the user does not have permission to access the table(s) referenced by the query.

But from the way it's treated in places in the class, I wouldn't be surprised if in certain PHP environments, it becomes a Boolean.

Otomatic 2015-06-21 12:38:11

I modify the function like this

	function close()
	{
		if ($this->link_id)
		{
			if ($this->query_result) {
				if ($this->query_result === false) {
					echo 'This is false';
					$this->saved_queries[] = array("false", sprintf('%.5f', 0));
					++$this->num_queries;
				}
				elseif ($this->query_result === true){
					echo 'This is true';
					$this->saved_queries[] = array("true", sprintf('%.5f', 0));
					++$this->num_queries;
				}
				//@mysqli_free_result($this->query_result);
			}

			return @mysqli_close($this->link_id);
		}
		else
			return false;
	}

I get "This is true".
I think this is the result of the last query UPDATE:

Informations de débogage
Temps (s) 	Requête
0.00020 	SET NAMES 'utf8'
0.00068 	SELECT u.*, g.*, o.logged, o.idle FROM fluxbb_users AS u INNER JOIN fluxbb_groups AS g ON u.group_id=g.g_id LEFT JOIN fluxbb_online AS o ON o.user_id=u.id WHERE u.id=2
0.00040 	UPDATE fluxbb_online SET logged=1434890165 WHERE user_id=2
0.00057 	SELECT user_id, ident, logged, idle FROM fluxbb_online WHERE logged<1434889565
0.00097 	SELECT t.subject, t.closed, t.num_replies, t.sticky, t.first_post_id, f.id AS forum_id, f.forum_name, f.moderators, fp.post_replies, s.user_id AS is_subscribed FROM fluxbb_topics AS t INNER JOIN fluxbb_forums AS f ON f.id=t.forum_id LEFT JOIN fluxbb_topic_subscriptions AS s ON (t.id=s.topic_id AND s.user_id=2) LEFT JOIN fluxbb_forum_perms AS fp ON (fp.forum_id=f.id AND fp.group_id=1) WHERE (fp.read_forum IS NULL OR fp.read_forum=1) AND t.id=320 AND t.moved_to IS NULL
  	SELECT id FROM fluxbb_posts WHERE topic_id=320 ORDER BY id DESC LIMIT 0,20
  	SELECT u.email, u.title, u.url, u.location, u.signature, u.email_setting, u.num_posts, u.registered, u.admin_note, p.id, p.poster AS username, p.poster_id, p.poster_ip, p.poster_email, p.message, p.hide_smilies, p.posted, p.edited, p.edited_by, g.g_id, g.g_user_title, g.g_promote_next_group, o.user_id AS is_online FROM fluxbb_posts AS p INNER JOIN fluxbb_users AS u ON u.id=p.poster_id INNER JOIN fluxbb_groups AS g ON g.g_id=u.group_id LEFT JOIN fluxbb_online AS o ON (o.user_id=u.id AND o.user_id!=1 AND o.idle=0) WHERE p.id IN (6974,6973,6972,6971,6970,6969,6968,6967,6966,6965,6963,6961,6959,6958,6957,6956,6955,6952,6951,6950) ORDER BY p.id DESC
  	UPDATE fluxbb_topics SET num_views=num_views+1 WHERE id=320
Temps total d'exécution de la requête : 0,00282 s

chris98 2015-06-21 12:56:09

I think this is the result of the last query UPDATE:

If this is the case (which I'm honestly not sure), I'm very surprised this did not happen in the past.

You can test this by checking if the previous query result is equal to Boolean true.

Comment edited 3 times (Diff, Diff 2, Diff 3)

Otomatic 2015-06-21 13:31:12

From MySQL documentation (http://dev.mysql.com/doc/refman/5.6/en/update.html)
For MySQL 5.0.x, 5.1.x, 5.5.x and 5.6.x
UPDATE returns the number of rows that were actually changed.

May be the return of "true" is due to a bug or a modification of MySQL 5.0.11 with Free.

chris98 2015-06-21 13:33:38

But from the manual though, it says this:

For other type of SQL statements, INSERT, UPDATE, DELETE, DROP, etc, mysql_query() returns TRUE on success or FALSE on error.

It looks like more of a bug in the way it's used rather than in MySQL. That's pretty much exactly what they're using here - take a look at this:

	function query($sql, $unbuffered = false)
	{
		if (defined('PUN_SHOW_QUERIES'))
			$q_start = get_microtime();

		$this->query_result = @mysqli_query($this->link_id, $sql);

		if ($this->query_result)
		{
			if (defined('PUN_SHOW_QUERIES'))
				$this->saved_queries[] = array($sql, sprintf('%.5f', get_microtime() - $q_start));

			++$this->num_queries;

			return $this->query_result;
		}
		else
		{
			if (defined('PUN_SHOW_QUERIES'))
				$this->saved_queries[] = array($sql, 0);

			$this->error_no = @mysqli_errno($this->link_id);
			$this->error_msg = @mysqli_error($this->link_id);

			return false;
		}
	}

They're just overwriting the value of $this->query_result.

Comment edited 1 times (Diff)

Otomatic 2015-06-21 14:27:33

There is difference between MySQL manual and PHP manual for the value returned with UPDATE query.

if ($this->query_result) does not test if the value is explicitly "true", but if it is equivalent to "true" and lots of "stuff" may implicitly assert "true".
See http://php.net/manual/en/language.types … an.casting

Modify mysqli.php:
- Add var $last_query

	var $saved_queries = array();
	var $num_queries = 0;
	var $last_query = '';

- Modify function query

			if (defined('PUN_SHOW_QUERIES')) {
				$this->saved_queries[] = array($sql, sprintf('%.5f', get_microtime() - $q_start));
				$this->last_query = $sql;
			}
			++$this->num_queries;

- Modify function close

	function close()
	{
		if ($this->link_id)
		{
			if (is_bool($this->query_result)) {
				if ($this->query_result === false) {
					if (defined('PUN_SHOW_QUERIES'))
						echo 'This is false<br>Last query:<br>'.$this->last_query;
				}
				elseif ($this->query_result === true){
					if (defined('PUN_SHOW_QUERIES'))
						echo 'This is true<br>Last query:<br>'.$this->last_query;
				}
			}
			else
				@mysqli_free_result($this->query_result);

			return @mysqli_close($this->link_id);
		}
		else
			return false;
	}

The boolean "true" value is always for UPDATE:
This is true
Last query:
UPDATE fluxbb_topics SET num_views=num_views+1 WHERE id=320

Otomatic 2015-06-22 07:58:29

I continued testing locally and at Gandi and Free and I arrives at the following conclusion:
- The Warning was present from the start, with anyone version of PHP 5, as explained in the PHP documentation:
"If the value passed to the argument result is not a resource, E_WARNING will be emitted "
But, it was masked by @:
  @mysqli_free_result ($this->query_result);

So the question is:
- Why a masked Warning is it transforms into a Fatal Error with PHP 5.6.8 alpha test at Free ?

Nevertheless it is better to change the function as I previously indicated by testing whether the last result is a boolean.

Comment edited 1 times (Diff)

chris98 2015-06-22 08:03:45

The problem is though, that doesn't change the fact that it's a Boolean. It's a Boolean for a reason, and just checking if it's not a Boolean doesn't change that fact something went wrong.

Clearly though, supressing it doesn't do anything.

Otomatic 2015-06-24 13:50:49

Although "Free" comes to modify files to generate a "Warning" and not a "Fatal Error", the "patch" that I mentioned is not only necessary to avoid this "Warning", but, in addition, it corresponds to a PHP code writing more standards compliant.

Indeed, the last request made when reading a discussion page is:

UPDATE fluxbb_topics SET num_views=num_views+1 WHERE id=xxx

Now, according to the PHP documentation, the return of an UPDATE query is "true" if all went well and "false" if problem:

« For SELECT, SHOW, DESCRIBE, EXPLAIN and other statements returning resultset, mysql_query() returns a resource on success, or FALSE on error.
For other type of SQL statements, INSERT, UPDATE, DELETE, DROP, etc, mysql_query() returns TRUE on success or FALSE on error. »

So it seems to me entirely normal and necessary to check what the result of the last request and not to do mysqli_free_result() if the result is boolean.
I persevere in advocating for change related close functions "dblayer" for MySQL as below:

	function close()
	{
		if ($this->link_id)
		{
			if (!is_bool($this->query_result))
				@mysqli_free_result($this->query_result);

			return @mysqli_close($this->link_id);
		}
		else
			return false;
	}

chris98 2015-06-24 14:55:02

I think it would be better to deal with it at the source - namely the query() method.

There you can check if the result is a Boolean, and if it isn't, then assign the result variable to it.

Otomatic 2015-06-24 15:44:04

chris98 wrote:

There you can check if the result is a Boolean, and if it isn't, then assign the result variable to it

I don't understand. This will not prevent the result of the last request before the close of being a boolean since the latter query is an UPDATE.

quy 2015-07-06 15:55:37

Please check if this will work:

if (is_object($this->query_result))

Otomatic 2015-07-07 16:46:53

Hi,

Sorry for the delay, but I have done extensive testing.
with

if (is_object($this->query_result))

it works perfectly.

Comment edited 1 times (Diff)

quy 2015-10-05 14:54:28

Otomatic: Please try this:

if ($this->query_result instanceof mysqli_result)

quy 2015-10-05 21:50:11

Commit f92c70b to fluxbb 1.5-next

#1041 Check that the result is a valid resource

Franz 2015-10-12 14:35:13

Commit 5b3bf09 to fluxbb 1.5-next

Merge pull request #160 from Quy/1041-query_result

#1041 Check that the result is a valid resource

quy 2015-10-12 17:35:50

  • Owner set to quy.
  • Status changed from open to fixed.

Visman 2015-10-14 05:29:48

mysql_innodb.php and mysqli_innodb.php?

Otomatic 2015-10-14 07:27:57

Hi,

if ($this->query_result instanceof mysqli_result)

It works !

Visman 2015-11-01 03:17:53

Add this to mysql_innodb.php and mysqli_innodb.php files.

Comment edited 1 times (Diff)

quy 2015-11-01 17:07:12

Commit 93da8c2 to fluxbb 1.5-next

#1041 Check that the result is a valid resource in innodb

oldskool 2015-11-02 08:45:03

Commit 6f2e3d6 to fluxbb 1.5-next

Merge pull request #162 from Quy/1041-innodb

#1041 Check that the result is a valid resource in innodb

quy 2015-11-05 10:47:38

Commit 253cd19 to fluxbb master

#1041 Check that the result is a valid resource

quy 2015-11-05 10:48:32

Commit 5dff00a to fluxbb master

#1041 Check that the result is a valid resource in innodb