Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERROR: store_buffer: Condition ' !p_src ' is true when doing some operations. #33564

Closed
qarmin opened this issue Nov 12, 2019 · 30 comments · Fixed by #49394
Closed

ERROR: store_buffer: Condition ' !p_src ' is true when doing some operations. #33564

qarmin opened this issue Nov 12, 2019 · 30 comments · Fixed by #49394

Comments

@qarmin
Copy link
Contributor

qarmin commented Nov 12, 2019

Godot version:
3.2.beta.custom_build. 157246a
OS/device including version:
Ubuntu 19.10
Issue description:

Using null buffer pointer in fwrite function is Undefinied Behaviour

size_t fwrite(
   const void *buffer,
   size_t size,
   size_t count,
   FILE *stream
);

and this cause this error

ERROR: store_buffer: Condition ' !p_src ' is true.
   At: drivers/unix/file_access_unix.cpp:278.

It can be easily silenced but probably this is not the best solution

Steps to reproduce:

  1. Download "FirstPersonStarter" from Template in Project Manager

file->store_buffer(r.ptr(), chunk.size());

2.
Import GLTF File Katalog bez nazwy.zip

f->store_buffer(r.ptr(), len);

@jameswestman
Copy link
Contributor

I'm also seeing this issue when downloading files with the HTTPRequest node. The download still works, though.

@git2013vb
Copy link

git2013vb commented Jan 3, 2020

I have the exactly the same error when I use this plugin: https://github.com/fenix-hub/godot-engine.text-editor
I use 3.2beta4.mono with Debian 10

ERROR: store_buffer: Condition ' !p_src ' is true.
   At: drivers/unix/file_access_unix.cpp:278.
ERROR: store_buffer: Condition ' !p_src ' is true.
   At: drivers/unix/file_access_unix.cpp:278.

@Touitoui
Copy link

Touitoui commented Jun 5, 2020

I have the same error. I don't have any plugin and i don't use HTTPRequest but i do have GLTF files imported from blender. (Created from scratch and exported by me with the latest version of blender).

It always happen in the script editor when i have an "error" in my code (ex: when writting an "if" before putting the ":" at the end).

It freeze the whole program for a few minutes... But if godot was started from a terminal, i can do a ctrl+c in it and godot will unfreeze like nothing happened ! Not exiting the program, just unfreezing it.

I use Godot_v3.2.1-stable_x11.64 on Ubuntu 18.04.4 LTS

@dpensky
Copy link

dpensky commented Mar 23, 2021

I also have this problem every time I use http_request to download a file on the android exported version of my game.
Using Engine: 3.2.3-stable (official)
Engine running on Ubuntu 16.04
Exported game running on Android 9
The download works correctly, only the debug log is too flooded:

godot-error

@avnerh1
Copy link

avnerh1 commented Apr 20, 2021

This solved the problem in my case:
$HTTPRequest.download_chunk_size = 65535

@Calinou
Copy link
Member

Calinou commented Apr 20, 2021

This solved the problem in my case:
$HTTPRequest.download_chunk_size = 65535

For reference, this is now the default in 3.3rc: #42896

@avnerh1
Copy link

avnerh1 commented Apr 23, 2021

This solved the problem in my case:
$HTTPRequest.download_chunk_size = 65535

For reference, this is now the default in 3.3rc: #42896

In 3.3, this doesn't solve the problem.
And although the file is still downloaded successfully, HTTPRequest.get_body_size() returns -1 instead of the actual file size.

@Calinou
Copy link
Member

Calinou commented Apr 23, 2021

HTTPRequest.get_body_size() returns -1 instead of the actual file size.

This is expected with some file hosts such as GitHub because they don't report the body size for files that were generated on-the-fly (such as ZIP archives). In this situation, you will get the body size if you try downloading the same file a second time in a short enough interval.

@avnerh1
Copy link

avnerh1 commented Apr 23, 2021

HTTPRequest.get_body_size() returns -1 instead of the actual file size.

This is expected with some file hosts such as GitHub because they don't report the body size for files that were generated on-the-fly (such as ZIP archives). In this situation, you will get the body size if you try downloading the same file a second time in a short enough interval.

That's generally true, but in my case these are the same specific files of which get_body_size() returns the actual size when exporting with Godot v3.2.3 but not when exporting with v3.3.

@Calinou
Copy link
Member

Calinou commented Apr 23, 2021

@avnerh1 Which platform are you testing this with? The HTML5 HTTPClient has been rewritten to use fetch(), so it's possible that there may be regressions. Please create a new issue with a minimal reproduction project attached.

@volzhs
Copy link
Contributor

volzhs commented Apr 23, 2021

I got similar error when running on android device via editor. 305c364
Screenshot_20210424_015401

@avnerh1
Copy link

avnerh1 commented Apr 25, 2021

@avnerh1 Which platform are you testing this with? The HTML5 HTTPClient has been rewritten to use fetch(), so it's possible that there may be regressions. Please create a new issue with a minimal reproduction project attached.

I'm using Win10/Edge.

@tavurth
Copy link
Contributor

tavurth commented May 3, 2021

I'm having this issue on Mac OS X.

The problem occurs in both debug mode (editor) and exported versions.

Screenshot 2021-05-03 at 18 18 50

Screenshot 2021-05-03 at 18 19 03

const endpoint = "https://query1.finance.yahoo.com/v8/finance/chart/AAPL?symbol=AAPL&period1=1614957299&period2=1620054899&useYfid=true&interval=15m&includePrePost=false"
const headers = [
	"Content-Type: application/json",
	"Accept-Encoding: gzip"
]

var http
http = HTTPRequest.new()
http.download_chunk_size = 65535
http.download_file = "user://test.gz"

self.add_child(http)

var result = http.request(endpoint, headers, true)
assert(result == OK)

result = yield(http, "request_completed")

Screenshot 2021-05-03 at 18 20 24

@dpensky
Copy link

dpensky commented May 13, 2021

HTTPRequest.get_body_size() returns -1 instead of the actual file size.

This is expected with some file hosts such as GitHub because they don't report the body size for files that were generated on-the-fly (such as ZIP archives). In this situation, you will get the body size if you try downloading the same file a second time in a short enough interval.

That's generally true, but in my case these are the same specific files of which get_body_size() returns the actual size when exporting with Godot v3.2.3 but not when exporting with v3.3.

I am also having this same problem. It worked in version 3.2.3 and in version 3.3 the function get_body_size () always returns -1 in the same file and server that returned correctly in the version exported with 3.2.3.
Does anyone have any fix or workaround for this? I used this function to show a progress bar.

var download_url = 'https://myawsserver.com/game.pck'
var download_destination = "user://game.pck"
download_request = HTTPRequest.new()
add_child(download_request)
# download_request.download_chunk_size = 65535;
download_request.download_file = download_destination
download_request.connect("request_completed", self, "_http_request_completed")
var error = download_request.request(download_url)

func _process(_delta):
  if download_request:
    var size = download_request.get_body_size()
    if size > 0:
      var done = float(download_request.get_downloaded_bytes()) / size

I have full access to the server (AWS) and the file. Is there any configuration that can solve this?

File is a pck with about 20Mb. File is downloaded correctly, buy progress bar stoped working

@tavurth
Copy link
Contributor

tavurth commented May 13, 2021

but progress bar stoped working @dpensky

It's probably not working because the server on the other end is responding with -1 as the content length.

If you could make a small repo to test that it would help narrow down the issue a lot!

@dpensky
Copy link

dpensky commented May 14, 2021

Running directly in the editor, and downloading the same file from the same location, everything works perfectly, it only happens when I export to html

@dpensky
Copy link

dpensky commented May 14, 2021

Log when running from the EDITOR (this works)

[pck_loader] - Downloading: [https://server.address.com:8080/game_name.pck]
[pck_loader] - 	To: [user://game_name.pck]
[pck_loader] - _http_request_completed(0, 200, [Server: TornadoServer/6.1, Content-Type: application/octet-stream, Date: Fri, 14 May 2021 11:17:39 GMT, Accept-Ranges: bytes, Etag: "0fe59c74512f93f843904cc9a2ff201ffa838967e28a1bd8014258a36a11cc702f50f19ce32a9546575cfa727ef3ac2b59bd9451ff43f67af07f84090c1a6197", Last-Modified: Thu, 13 May 2021 19:58:15 GMT, Content-Length: 15809392], )

Log when running from the HTML EXPORT (this don't work)

[pck_loader] - Downloading: [https://server.address.com:8080/game_name.pck]
[pck_loader] - 	To: [user://game_name.pck]
**ERROR**: Condition "!p_src" is true.
game-lobby.js?v=…2b1d2ad644eabc5:354    At: drivers/unix/file_access_unix.cpp:279:store_buffer() - Condition "!p_src" is true.
game-lobby.js?v=…2b1d2ad644eabc5:354 **ERROR**: Condition "!p_src" is true.
game-lobby.js?v=…2b1d2ad644eabc5:354    At: drivers/unix/file_access_unix.cpp:279:store_buffer() - Condition "!p_src" is true.
[pck_loader] - _http_request_completed(0, 200, [Server: TornadoServer/6.1, Content-Type: application/octet-stream, Date: Fri, 14 May 2021 11:17:39 GMT, Accept-Ranges: bytes, Etag: "0fe59c74512f93f843904cc9a2ff201ffa838967e28a1bd8014258a36a11cc702f50f19ce32a9546575cfa727ef3ac2b59bd9451ff43f67af07f84090c1a6197", Last-Modified: Thu, 13 May 2021 19:58:15 GMT, Content-Length: 15809392], )

@yuyutsupoudel
Copy link

Has anyone found solution for this error? Im trying to download files using httprequest node but only sometimes issue is present.

@Calinou
Copy link
Member

Calinou commented May 28, 2021

Running directly in the editor, and downloading the same file from the same location, everything works perfectly, it only happens when I export to html

This is likely a regression from #46731 (see also #47597). cc @Faless

Has anyone found solution for this error? Im trying to download files using httprequest node but only sometimes issue is present.

Which operating system and Godot version are you using? If this is HTML5, see above.

@yuyutsupoudel
Copy link

Which operating system and Godot version are you using? If this is HTML5, see above.

Im using popos with godot 3.3.2. Error seems to come off at random interval. Tested with filesize less than 1 mb, issue isn't present. Tested with 10mb file issue is present.

@Calinou
Copy link
Member

Calinou commented May 28, 2021

Im using popos with godot 3.3.2. Error seems to come off at random interval. Tested with filesize less than 1 mb, issue isn't present. Tested with 10mb file issue is present.

Which website are you downloading the file from? Many hosts such as GitHub will not report content lengths unless the file was already recently downloaded. This is because the file is generated on-the-fly, so GitHub doesn't even know the file size until the file is generated.

@yuyutsupoudel
Copy link

Mainly itch io and various other sites. Including couple of pictures from wikipedia. Although error is present, file is downloaded.

@avnerh1
Copy link

avnerh1 commented May 28, 2021

I noticed that the problem is not consistent. I suspect it has something to do with caching the file by CDN. I will look into it and report back if I find anything useful.

@darcio-studiobluechip
Copy link

For me the error of httprequet.get_body_size () returning -1 always happens in version 3.3 (and 3.3.2)

This function returns the correct value in version 3.2.3 when downloading the same file, on the same server in the same game

@tavurth
Copy link
Contributor

tavurth commented May 28, 2021

@darcio-studiobluechip which file? Could you provide a sample project?

@darcio-studiobluechip
Copy link

HTTPRequest.get_body_size() returns -1 instead of the actual file size.

This is expected with some file hosts such as GitHub because they don't report the body size for files that were generated on-the-fly (such as ZIP archives). In this situation, you will get the body size if you try downloading the same file a second time in a short enough interval.

That's generally true, but in my case these are the same specific files of which get_body_size() returns the actual size when exporting with Godot v3.2.3 but not when exporting with v3.3.

I am also having this same problem. It worked in version 3.2.3 and in version 3.3 the function get_body_size () always returns -1 in the same file and server that returned correctly in the version exported with 3.2.3.
Does anyone have any fix or workaround for this? I used this function to show a progress bar.

var download_url = 'https://myawsserver.com/game.pck'
var download_destination = "user://game.pck"
download_request = HTTPRequest.new()
add_child(download_request)
# download_request.download_chunk_size = 65535;
download_request.download_file = download_destination
download_request.connect("request_completed", self, "_http_request_completed")
var error = download_request.request(download_url)

func _process(_delta):
  if download_request:
    var size = download_request.get_body_size()
    if size > 0:
      var done = float(download_request.get_downloaded_bytes()) / size

I have full access to the server (AWS) and the file. Is there any configuration that can solve this?

File is a pck with about 20Mb. File is downloaded correctly, buy progress bar stoped working

My code is the above one.

  • file is a godot game (only pck)
  • host is in a amazon instance (s3)
  • webserver is python tornado
  • http exported game and files are stored in the same place/server/instance
  • file is always downloaded correctly
  • get_body_size works when running from the engine editor

@dpensky
Copy link

dpensky commented May 31, 2021

get_body_size also works when running the same code and requesting the same file from the same server on the android exported version of the game

@avnerh1
Copy link

avnerh1 commented Jun 7, 2021

Just switched from v3.2.3 to v3.3.2 and with HTML export the problem immediately showed up again, for every file.
Do we know the root cause of the problem, and what was changed from 3.2 to 3.3 that caused it?

@Calinou
Copy link
Member

Calinou commented Jun 7, 2021

Just switched from v3.2.3 to v3.3.2 and with HTML export the problem immediately showed up again, for every file.
Do we know the root cause of the problem, and what was changed from 3.2 to 3.3 that caused it?

Quoting myself above:

This is likely a regression from #46731 (see also #47597).

However, it was recently fixed by #49226 which will be available in 3.4.

@akien-mga akien-mga added this to the 4.0 milestone Jun 7, 2021
akien-mga added a commit to akien-mga/godot that referenced this issue Jun 7, 2021
The error check was added for `FileAccessUnix` but it's not an error when both
`p_src` and `p_length` are zero.

Added correct error checks to all implementations to prevent the actual
erroneous case: `p_src` is nullptr but `p_length > 0` (risk of null pointer
indexing).

Fixes godotengine#33564.
@akien-mga
Copy link
Member

akien-mga commented Jun 7, 2021

The fix is simple:

diff --git a/drivers/unix/file_access_unix.cpp b/drivers/unix/file_access_unix.cpp
index ec23df62d0..6ea55219bb 100644
--- a/drivers/unix/file_access_unix.cpp
+++ b/drivers/unix/file_access_unix.cpp
@@ -264,7 +264,7 @@ void FileAccessUnix::store_8(uint8_t p_dest) {

 void FileAccessUnix::store_buffer(const uint8_t *p_src, uint64_t p_length) {
        ERR_FAIL_COND_MSG(!f, "File must be opened before use.");
-       ERR_FAIL_COND(!p_src);
+       ERR_FAIL_COND(!p_src && p_length > 0);
        ERR_FAIL_COND(fwrite(p_src, 1, p_length, f) != p_length);
 }
 

Cf. #49394.

For what it's worth, the errors are harmless, it's just noise. It errors on code that tries to store a non-existent buffer of size 0... which doesn't need to be an error, it will just not do anything.

akien-mga added a commit that referenced this issue Jun 7, 2021
The error check was added for `FileAccessUnix` but it's not an error when both
`p_src` and `p_length` are zero.

Added correct error checks to all implementations to prevent the actual
erroneous case: `p_src` is nullptr but `p_length > 0` (risk of null pointer
indexing).

Fixes #33564.

(cherry picked from commit 01d5c46)
akien-mga added a commit that referenced this issue Jun 7, 2021
The error check was added for `FileAccessUnix` but it's not an error when both
`p_src` and `p_length` are zero.

Added correct error checks to all implementations to prevent the actual
erroneous case: `p_src` is nullptr but `p_length > 0` (risk of null pointer
indexing).

Fixes #33564.

(cherry picked from commit 01d5c46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.